Skip to content

Commit a9f1645

Browse files
yrumaxmb388a
authored andcommitted
Allow specific version to be deleted from cache
You can now specify a specific ref to be removed from the cache rather than having to remove all of them
1 parent 84da9c5 commit a9f1645

File tree

10 files changed

+131
-62
lines changed

10 files changed

+131
-62
lines changed

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
202202
FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName());
203203
Boolean shouldCache = cachingConfiguration != null;
204204
final FilePath versionCacheDir = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), record.getDirectoryName());
205-
ReentrantReadWriteLock retrieveLock = getReadWriteLockFor(record.getDirectoryName());
205+
ReentrantReadWriteLock retrieveLock = getReadWriteLockFor(record.getName());
206206
final FilePath lastReadFile = new FilePath(versionCacheDir, LibraryCachingConfiguration.LAST_READ_FILE);
207207

208208
if(shouldCache && cachingConfiguration.isExcluded(version)) {
@@ -238,7 +238,7 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
238238
}
239239

240240
if (retrieve) {
241-
listener.getLogger().println("Caching library " + name + "@" + version);
241+
listener.getLogger().println("Caching library " + name + "@" + version);
242242
versionCacheDir.mkdirs();
243243
retriever.retrieve(name, version, changelog, versionCacheDir, run, listener);
244244
}
@@ -251,7 +251,7 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
251251
}
252252

253253
lastReadFile.touch(System.currentTimeMillis());
254-
versionCacheDir.withSuffix("-name.txt").write(name, "UTF-8");
254+
versionCacheDir.withSuffix("-name.txt").write(name + "@" + version, "UTF-8");
255255
versionCacheDir.copyRecursiveTo(libDir);
256256
} finally {
257257
retrieveLock.readLock().unlock();

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,13 @@ public LibraryCachingCleanup() {
2828
@Override protected void execute(TaskListener listener) throws IOException, InterruptedException {
2929
FilePath globalCacheDir = LibraryCachingConfiguration.getGlobalLibrariesCacheDir();
3030
for (FilePath library : globalCacheDir.list()) {
31-
if (!removeIfExpiredCacheDirectory(library)) {
32-
// Prior to the SECURITY-2586 fix, library caches had a two-level directory structure.
33-
// These caches will never be used again, so we delete any that we find.
34-
for (FilePath version: library.list()) {
35-
if (version.child(LibraryCachingConfiguration.LAST_READ_FILE).exists()) {
36-
library.deleteRecursive();
37-
break;
31+
for (FilePath versionDir : library.listDirectories()) {
32+
if (!removeIfExpiredCacheDirectory(versionDir)) {
33+
FilePath parent = versionDir.getParent();
34+
if (parent != null) {
35+
parent.deleteRecursive();
3836
}
37+
break;
3938
}
4039
}
4140
}
@@ -48,14 +47,15 @@ public LibraryCachingCleanup() {
4847
*/
4948
private boolean removeIfExpiredCacheDirectory(FilePath library) throws IOException, InterruptedException {
5049
final FilePath lastReadFile = new FilePath(library, LibraryCachingConfiguration.LAST_READ_FILE);
51-
if (lastReadFile.exists()) {
50+
if (lastReadFile.exists() && library.withSuffix("-name.txt").exists()) {
5251
ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(library.getName());
5352
retrieveLock.writeLock().lockInterruptibly();
5453
try {
5554
if (System.currentTimeMillis() - lastReadFile.lastModified() > TimeUnit.DAYS.toMillis(EXPIRE_AFTER_READ_DAYS)) {
56-
57-
library.deleteRecursive();
58-
library.withSuffix("-name.txt").delete();
55+
FilePath parent = library.getParent();
56+
if (parent != null) {
57+
parent.deleteRecursive();
58+
}
5959
}
6060
} finally {
6161
retrieveLock.writeLock().unlock();

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -89,46 +89,61 @@ public static FilePath getGlobalLibrariesCacheDir() {
8989
}
9090

9191
@Extension public static class DescriptorImpl extends Descriptor<LibraryCachingConfiguration> {
92-
public FormValidation doClearCache(@QueryParameter String name, @QueryParameter boolean forceDelete) throws InterruptedException {
92+
public FormValidation doClearCache(@QueryParameter String name, @QueryParameter String cachedLibraryRef, @QueryParameter boolean forceDelete) throws InterruptedException {
9393
Jenkins.get().checkPermission(Jenkins.ADMINISTER);
94-
94+
String cacheDirName = null;
9595
try {
9696
if (LibraryCachingConfiguration.getGlobalLibrariesCacheDir().exists()) {
97-
for (FilePath libraryNamePath : LibraryCachingConfiguration.getGlobalLibrariesCacheDir().list("*-name.txt")) {
98-
// Libraries configured in distinct locations may have the same name. Since only admins are allowed here, this is not a huge issue, but it is probably unexpected.
99-
String cacheName;
100-
try (InputStream stream = libraryNamePath.read()) {
101-
cacheName = IOUtils.toString(stream, StandardCharsets.UTF_8);
102-
}
103-
if (cacheName.equals(name)) {
104-
FilePath libraryCachePath = LibraryCachingConfiguration.getGlobalLibrariesCacheDir()
105-
.child(libraryNamePath.getName().replace("-name.txt", ""));
106-
if (forceDelete) {
107-
LOGGER.log(Level.FINER, "Force deleting cache for {0}", name);
108-
libraryCachePath.deleteRecursive();
109-
libraryNamePath.delete();
110-
} else {
111-
LOGGER.log(Level.FINER, "Safe deleting cache for {0}", name);
112-
ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(libraryCachePath.getName());
113-
if (retrieveLock.writeLock().tryLock(10, TimeUnit.SECONDS)) {
114-
try {
115-
libraryCachePath.deleteRecursive();
116-
libraryNamePath.delete();
117-
} finally {
118-
retrieveLock.writeLock().unlock();
97+
outer: for (FilePath libraryCache : LibraryCachingConfiguration.getGlobalLibrariesCacheDir().listDirectories()) {
98+
for (FilePath libraryNamePath : libraryCache.list("*-name.txt")) {
99+
if (libraryNamePath.readToString().startsWith(name + "@")) {
100+
FilePath libraryCachePath = libraryNamePath.getParent();
101+
if (libraryCachePath != null) {
102+
FilePath versionCachePath = new FilePath(libraryCachePath, libraryNamePath.getName().replace("-name.txt", ""));
103+
LOGGER.log(Level.FINER, "Safe deleting cache for {0}", name);
104+
ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(libraryCachePath.getName());
105+
if (forceDelete || retrieveLock.writeLock().tryLock(10, TimeUnit.SECONDS)) {
106+
if (forceDelete) {
107+
LOGGER.log(Level.FINER, "Force deleting cache for {0}", name);
108+
} else {
109+
LOGGER.log(Level.FINER, "Safe deleting cache for {0}", name);
110+
}
111+
try {
112+
if (StringUtils.isNotEmpty(cachedLibraryRef)) {
113+
if (libraryNamePath.readToString().equals(name + "@" + cachedLibraryRef)) {
114+
cacheDirName = name + "@" + cachedLibraryRef;
115+
libraryNamePath.delete();
116+
versionCachePath.deleteRecursive();
117+
break outer;
118+
}
119+
} else {
120+
cacheDirName = name;
121+
libraryCachePath.deleteRecursive();
122+
break outer;
123+
}
124+
} finally {
125+
if (!forceDelete) {
126+
retrieveLock.writeLock().unlock();
127+
}
128+
}
129+
} else {
130+
return FormValidation.error("The cache dir could not be deleted because it is currently being used by another thread. Please try again.");
119131
}
120-
} else {
121-
return FormValidation.error("The cache dir could not be deleted because it is currently being used by another thread. Please try again.");
122132
}
123133
}
124134
}
125135
}
126136
}
127137
} catch (IOException ex) {
128-
return FormValidation.error(ex, "The cache dir was not deleted successfully");
138+
return FormValidation.error(ex, String.format("The cache dir %s was not deleted successfully", cacheDirName));
139+
}
140+
141+
if (cacheDirName == null) {
142+
return FormValidation.ok(String.format("The version %s was not found for library %s.", cachedLibraryRef, name));
143+
} else {
144+
return FormValidation.ok(String.format("The cache dir %s was deleted successfully.", cacheDirName));
129145
}
130-
return FormValidation.ok("The cache dir was deleted successfully.");
131146
}
132147

133148
}
134-
}
149+
}

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRecord.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
package org.jenkinsci.plugins.workflow.libs;
2626

27+
import java.io.File;
2728
import java.util.Collections;
2829
import java.util.Set;
2930
import java.util.TreeSet;
@@ -62,7 +63,7 @@ public final class LibraryRecord {
6263
this.trusted = trusted;
6364
this.changelog = changelog;
6465
this.cachingConfiguration = cachingConfiguration;
65-
this.directoryName = directoryNameFor(name, version, String.valueOf(trusted), source);
66+
this.directoryName = directoryNameFor(name, String.valueOf(trusted), source) + File.separator + directoryNameFor(version);
6667
}
6768

6869
@Exported

src/main/resources/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration/config.jelly

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,12 @@ THE SOFTWARE.
3232
<f:textbox />
3333
</f:entry>
3434
<j:if test="${h.hasPermission(app.ADMINISTER)}">
35-
<f:entry title="${%Force clear cache}" field="forceDelete">
35+
<f:entry title="${%Clear cache for ref}" field="cachedLibraryRef">
36+
<f:textbox />
37+
</f:entry>
38+
<f:entry title="${%Force clear cache}" field="forceDelete">
3639
<f:checkbox/>
3740
</f:entry>
38-
<f:validateButton title="${%Clear cache}" progress="${%Clearing...}" method="clearCache" with="name,forceDelete" />
41+
<f:validateButton title="${%Clear cache}" progress="${%Clearing...}" method="clearCache" with="name,cachedLibraryRef,forceDelete" />
3942
</j:if>
4043
</j:jelly>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<div>
2+
Specifies a specific version to clear the cache for. An empty value will clear the cache for all versions.
3+
</div>

src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanupTest.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,16 @@ public void smokes() throws Throwable {
7171
assertThat(new File(cache.getRemote()), anExistingDirectory());
7272
// Run LibraryCachingCleanup and show that cache is not deleted.
7373
ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr());
74+
assertThat(new File(cache.getParent().getRemote()), anExistingDirectory());
7475
assertThat(new File(cache.getRemote()), anExistingDirectory());
7576
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), anExistingFile());
7677
// Run LibraryCachingCleanup after modifying LAST_READ_FILE to be an old date and and show that cache is deleted.
7778
long oldMillis = ZonedDateTime.now().minusDays(LibraryCachingCleanup.EXPIRE_AFTER_READ_DAYS + 1).toInstant().toEpochMilli();
7879
cache.child(LibraryCachingConfiguration.LAST_READ_FILE).touch(oldMillis);
7980
ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr());
80-
assertThat(new File(cache.getRemote()), not(anExistingDirectory()));
81-
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingDirectory()));
81+
assertThat(new File(cache.getParent().getRemote()), not(anExistingDirectory()));
82+
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingFile()));
83+
8284
}
8385

8486
@Test

src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import hudson.ExtensionList;
2828
import hudson.FilePath;
2929
import java.io.File;
30+
import java.util.Arrays;
31+
import java.util.List;
3032
import jenkins.plugins.git.GitSCMSource;
3133
import jenkins.plugins.git.GitSampleRepoRule;
3234
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
@@ -157,29 +159,68 @@ public void isExcluded() {
157159

158160
@Test
159161
public void clearCache() throws Exception {
162+
List<FilePath> caches = setupLibraryCaches();
163+
FilePath cache = caches.get(0);
164+
FilePath cache2 = caches.get(1);
165+
assertThat("Must be different paths", cache, not(equalTo(cache2)));
166+
assertThat(new File(cache.getParent().getRemote()), anExistingDirectory());
167+
assertThat(new File(cache.getRemote()), anExistingDirectory());
168+
assertThat(new File(cache2.getRemote()), anExistingDirectory());
169+
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), anExistingFile());
170+
assertThat(cache.withSuffix("-name.txt").readToString(), equalTo("library@master"));
171+
assertThat(cache2.withSuffix("-name.txt").readToString(), equalTo("library@feature/something"));
172+
// Clear the cache. TODO: Would be more realistic to set up security and use WebClient.
173+
ExtensionList.lookupSingleton(LibraryCachingConfiguration.DescriptorImpl.class).doClearCache("library", "", false);
174+
assertThat(new File(cache.getParent().getRemote()), not(anExistingDirectory()));
175+
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingFile()));
176+
}
177+
178+
@Test
179+
public void clearCacheVersion() throws Exception {
180+
181+
List<FilePath> caches = setupLibraryCaches();
182+
FilePath cache = caches.get(0);
183+
FilePath cache2 = caches.get(1);
184+
assertThat(new File(cache.getRemote()), anExistingDirectory());
185+
// Clear the cache. TODO: Would be more realistic to set up security and use WebClient.
186+
ExtensionList.lookupSingleton(LibraryCachingConfiguration.DescriptorImpl.class).doClearCache("library", "master", false);
187+
assertThat(new File(cache.getParent().getRemote()), anExistingDirectory());
188+
assertThat(new File(cache.getRemote()), not(anExistingDirectory()));
189+
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingFile()));
190+
//Other cache has not been touched
191+
assertThat(new File(cache2.getRemote()), anExistingDirectory());
192+
assertThat(new File(cache2.withSuffix("-name.txt").getRemote()), anExistingFile());
193+
}
194+
195+
196+
private List<FilePath> setupLibraryCaches() throws Exception {
160197
sampleRepo.init();
161198
sampleRepo.write("vars/foo.groovy", "def call() { echo 'foo' }");
162199
sampleRepo.git("add", "vars");
163200
sampleRepo.git("commit", "--message=init");
201+
sampleRepo.git("branch", "feature/something");
164202
LibraryConfiguration config = new LibraryConfiguration("library",
165203
new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true)));
166204
config.setDefaultVersion("master");
167-
config.setImplicit(true);
205+
config.setImplicit(false);
168206
config.setCachingConfiguration(new LibraryCachingConfiguration(30, null));
207+
config.setAllowVersionOverride(true);
169208
GlobalLibraries.get().getLibraries().add(config);
170209
// Run build and check that cache gets created.
171210
WorkflowJob p = r.createProject(WorkflowJob.class);
172-
p.setDefinition(new CpsFlowDefinition("foo()", true));
211+
p.setDefinition(new CpsFlowDefinition("library identifier: 'library', changelog:false\n\nfoo()", true));
173212
WorkflowRun b = r.buildAndAssertSuccess(p);
213+
WorkflowJob p2 = r.createProject(WorkflowJob.class);
214+
p2.setDefinition(new CpsFlowDefinition("library identifier: 'library@feature/something', changelog:false\n\nfoo()", true));
215+
WorkflowRun b2 = r.buildAndAssertSuccess(p2);
174216
LibrariesAction action = b.getAction(LibrariesAction.class);
175217
LibraryRecord record = action.getLibraries().get(0);
218+
LibrariesAction action2 = b2.getAction(LibrariesAction.class);
219+
LibraryRecord record2 = action2.getLibraries().get(0);
220+
176221
FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record.getDirectoryName());
177-
assertThat(new File(cache.getRemote()), anExistingDirectory());
178-
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), anExistingFile());
179-
// Clear the cache. TODO: Would be more realistic to set up security and use WebClient.
180-
ExtensionList.lookupSingleton(LibraryCachingConfiguration.DescriptorImpl.class).doClearCache("library", false);
181-
assertThat(new File(cache.getRemote()), not(anExistingDirectory()));
182-
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingFile()));
222+
FilePath cache2 = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record2.getDirectoryName());
223+
224+
return Arrays.asList(cache, cache2);
183225
}
184-
185226
}

0 commit comments

Comments
 (0)