diff --git a/src/main/java/io/jenkins/plugins/artifact_manager_jclouds/JCloudsArtifactManager.java b/src/main/java/io/jenkins/plugins/artifact_manager_jclouds/JCloudsArtifactManager.java index 60cc3b1f..c4f3635f 100644 --- a/src/main/java/io/jenkins/plugins/artifact_manager_jclouds/JCloudsArtifactManager.java +++ b/src/main/java/io/jenkins/plugins/artifact_manager_jclouds/JCloudsArtifactManager.java @@ -48,7 +48,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.HashMap; -import java.util.Iterator; import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; @@ -58,6 +57,7 @@ import org.apache.http.client.methods.HttpGet; import org.jclouds.blobstore.BlobStore; import org.jclouds.blobstore.BlobStoreContext; +import org.jclouds.blobstore.BlobStores; import org.jclouds.blobstore.domain.Blob; import org.jclouds.blobstore.domain.StorageMetadata; import org.jclouds.blobstore.options.CopyOptions; @@ -274,9 +274,7 @@ public void clearAllStashes(TaskListener listener) throws IOException, Interrupt BlobStore blobStore = getContext().getBlobStore(); int count = 0; try { - Iterator it = new JCloudsVirtualFile.PageSetIterable(blobStore, provider.getContainer(), ListContainerOptions.Builder.prefix(stashPrefix).recursive()); - while (it.hasNext()) { - StorageMetadata sm = it.next(); + for (StorageMetadata sm : BlobStores.listAll(blobStore, provider.getContainer(), ListContainerOptions.Builder.prefix(stashPrefix).recursive())) { String path = sm.getName(); assert path.startsWith(stashPrefix); LOGGER.fine("deleting " + path); @@ -300,9 +298,7 @@ public void copyAllArtifactsAndStashes(Run to, TaskListener listener) thro BlobStore blobStore = getContext().getBlobStore(); int count = 0; try { - Iterator it = new JCloudsVirtualFile.PageSetIterable(blobStore, provider.getContainer(), ListContainerOptions.Builder.prefix(allPrefix).recursive()); - while (it.hasNext()) { - StorageMetadata sm = it.next(); + for (StorageMetadata sm : BlobStores.listAll(blobStore, provider.getContainer(), ListContainerOptions.Builder.prefix(allPrefix).recursive())) { String path = sm.getName(); assert path.startsWith(allPrefix); String destPath = getBlobPath(dest.key, path.substring(allPrefix.length())); diff --git a/src/main/java/io/jenkins/plugins/artifact_manager_jclouds/JCloudsVirtualFile.java b/src/main/java/io/jenkins/plugins/artifact_manager_jclouds/JCloudsVirtualFile.java index 669a9fb7..08305e51 100644 --- a/src/main/java/io/jenkins/plugins/artifact_manager_jclouds/JCloudsVirtualFile.java +++ b/src/main/java/io/jenkins/plugins/artifact_manager_jclouds/JCloudsVirtualFile.java @@ -40,21 +40,17 @@ import java.util.Date; import java.util.Deque; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.NoSuchElementException; -import java.util.Spliterator; -import java.util.Spliterators; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.StreamSupport; import jenkins.util.VirtualFile; import org.jclouds.blobstore.BlobStore; import org.jclouds.blobstore.BlobStoreContext; +import org.jclouds.blobstore.BlobStores; import org.jclouds.blobstore.domain.Blob; import org.jclouds.blobstore.domain.MutableBlobMetadata; -import org.jclouds.blobstore.domain.PageSet; import org.jclouds.blobstore.domain.StorageMetadata; import org.jclouds.blobstore.options.ListContainerOptions; import static org.jclouds.blobstore.options.ListContainerOptions.Builder.*; @@ -154,7 +150,7 @@ public boolean isDirectory() throws IOException { String keyS = key + "/"; CacheFrame frame = findCacheFrame(keyS); if (frame != null) { - LOGGER.log(Level.FINE, "cache hit on directory status of {0} / {1}", new Object[] {container, key}); + LOGGER.log(Level.FINER, "cache hit on directory status of {0} / {1}", new Object[] {container, key}); String relSlash = keyS.substring(frame.root.length()); // "" or "sub/dir/" return frame.children.keySet().stream().anyMatch(f -> f.startsWith(relSlash)); } @@ -168,7 +164,7 @@ public boolean isFile() throws IOException { if (frame != null) { String rel = key.substring(frame.root.length()); CachedMetadata metadata = frame.children.get(rel); - LOGGER.log(Level.FINE, "cache hit on file status of {0} / {1}", new Object[] {container, key}); + LOGGER.log(Level.FINER, "cache hit on file status of {0} / {1}", new Object[] {container, key}); return metadata != null; } LOGGER.log(Level.FINE, "checking file status {0} / {1}", new Object[] {container, key}); @@ -186,12 +182,12 @@ public boolean exists() throws IOException { * @return some blobs * @throws RuntimeException either now or when the stream is processed; wrap in {@link IOException} if desired */ - private Iterator listStorageMetadata(boolean recursive) throws IOException { + private Iterable listStorageMetadata(boolean recursive) throws IOException { ListContainerOptions options = prefix(key + "/"); if (recursive) { options.recursive(); } - return new PageSetIterable(getContext().getBlobStore(), getContainer(), options); + return BlobStores.listAll(getContext().getBlobStore(), getContainer(), options); } @Override @@ -199,7 +195,7 @@ public VirtualFile[] list() throws IOException { String keyS = key + "/"; CacheFrame frame = findCacheFrame(keyS); if (frame != null) { - LOGGER.log(Level.FINE, "cache hit on listing of {0} / {1}", new Object[] {container, key}); + LOGGER.log(Level.FINER, "cache hit on listing of {0} / {1}", new Object[] {container, key}); String relSlash = keyS.substring(frame.root.length()); // "" or "sub/dir/" return frame.children.keySet().stream(). // filenames relative to frame root filter(f -> f.startsWith(relSlash)). // those inside this dir @@ -210,7 +206,7 @@ public VirtualFile[] list() throws IOException { } VirtualFile[] list; try { - list = StreamSupport.stream(Spliterators.spliteratorUnknownSize(listStorageMetadata(false), Spliterator.ORDERED), false) + list = StreamSupport.stream(listStorageMetadata(false).spliterator(), false) .map(meta -> new JCloudsVirtualFile(provider, getContainer(), meta.getName().replaceFirst("/$", ""))) .toArray(VirtualFile[]::new); } catch (RuntimeException x) { @@ -232,7 +228,7 @@ public long length() throws IOException { if (frame != null) { String rel = key.substring(frame.root.length()); CachedMetadata metadata = frame.children.get(rel); - LOGGER.log(Level.FINE, "cache hit on length of {0} / {1}", new Object[] {container, key}); + LOGGER.log(Level.FINER, "cache hit on length of {0} / {1}", new Object[] {container, key}); return metadata != null ? metadata.length : 0; } LOGGER.log(Level.FINE, "checking length {0} / {1}", new Object[] {container, key}); @@ -247,7 +243,7 @@ public long lastModified() throws IOException { if (frame != null) { String rel = key.substring(frame.root.length()); CachedMetadata metadata = frame.children.get(rel); - LOGGER.log(Level.FINE, "cache hit on lastModified of {0} / {1}", new Object[] {container, key}); + LOGGER.log(Level.FINER, "cache hit on lastModified of {0} / {1}", new Object[] {container, key}); return metadata != null ? metadata.lastModified : 0; } LOGGER.log(Level.FINE, "checking modification time {0} / {1}", new Object[] {container, key}); @@ -274,65 +270,6 @@ public InputStream open() throws IOException { return getBlob().getPayload().openStream(); } - /** - * An Iterator for JClouds PageSet - */ - @Restricted(NoExternalUse.class) - static class PageSetIterable implements Iterator { - private final BlobStore blobStore; - private final String container; - private ListContainerOptions options; - private PageSet set; - private Iterator iterator; - - /** - * @throws RuntimeException either now or when iterating; wrap in {@link IOException} if desired - */ - PageSetIterable(@NonNull BlobStore blobStore, @NonNull String container, - @NonNull ListContainerOptions options) { - this.blobStore = blobStore; - this.container = container; - advanceList(options); - } - - @Override - public boolean hasNext() { - if (iterator.hasNext()) { - return true; - } - String marker = set.getNextMarker(); - if (marker == null) { - return false; - } - advanceList(options.afterMarker(marker)); - return iterator.hasNext(); - } - - @Override - public StorageMetadata next() { - if (hasNext()) { - return iterator.next(); - } else { - throw new NoSuchElementException(); - } - } - - /** - * Unsupported operation - */ - @Override - public void remove() { - throw new UnsupportedOperationException(); - } - - private void advanceList(ListContainerOptions options) { - LOGGER.log(Level.FINE, "listing {0}: {1}", new Object[] {container, options}); - this.options = options; - this.set = blobStore.list(container, options); - this.iterator = set.iterator(); - } - } - /** * Cache of metadata collected during {@link #run}. * Keys are {@link #container}. @@ -373,9 +310,7 @@ public V run(Callable callable) throws IOException { Map saved = new HashMap<>(); int prefixLength = key.length() + /* / */1; try { - Iterator it = listStorageMetadata(true); - while (it.hasNext()) { - StorageMetadata sm = it.next(); + for (StorageMetadata sm : listStorageMetadata(true)) { Long length = sm.getSize(); if (length != null) { Date lastModified = sm.getLastModified(); @@ -409,10 +344,8 @@ private Deque cacheFrames() { */ public static boolean delete(BlobStoreProvider provider, BlobStore blobStore, String prefix) throws IOException, InterruptedException { try { - Iterator it = new PageSetIterable(blobStore, provider.getContainer(), ListContainerOptions.Builder.prefix(prefix).recursive()); List paths = new ArrayList<>(); - while (it.hasNext()) { - StorageMetadata sm = it.next(); + for (StorageMetadata sm : BlobStores.listAll(blobStore, provider.getContainer(), ListContainerOptions.Builder.prefix(prefix).recursive())) { String path = sm.getName(); assert path.startsWith(prefix); paths.add(path); diff --git a/src/test/java/io/jenkins/plugins/artifact_manager_s3/JCloudsVirtualFileTest.java b/src/test/java/io/jenkins/plugins/artifact_manager_s3/JCloudsVirtualFileTest.java index 481caa6f..03831532 100644 --- a/src/test/java/io/jenkins/plugins/artifact_manager_s3/JCloudsVirtualFileTest.java +++ b/src/test/java/io/jenkins/plugins/artifact_manager_s3/JCloudsVirtualFileTest.java @@ -186,11 +186,6 @@ public void list() throws Exception { @Test @SuppressWarnings("deprecation") public void listGlob() throws Exception { - httpLogging.record(InvokeHttpMethod.class, Level.FINE); - httpLogging.capture(1000); - assertThat(subdir.list("*", null, true), containsInAnyOrder(weirdCharacters.getName(), vf.getName())); - // TODO more interesting to create a directory with enough files to exceed a single iterator page: - assertEquals("calls GetBucketLocation then ListBucket", 2, httpLogging.getRecords().size()); assertThat(subdir.list("**/**"), arrayContainingInAnyOrder(vf.getName(), weirdCharacters.getName())); assertArrayEquals(new String[] { vf.getName() }, subdir.list(tmpFile.getName().substring(0, 4) + "*")); assertArrayEquals(new String[0], subdir.list("**/something**")); @@ -198,6 +193,25 @@ public void listGlob() throws Exception { assertArrayEquals(new String[0], missing.list("**/**")); } + @Test + public void pagedListing() throws Exception { + for (int i = 0; i < 10; i++) { + String iDir = getPrefix() + "sprawling/i" + i + "/"; + for (int j = 0; j < 10; j++) { + for (int k = 0; k < 10; k++) { + blobStore.putBlob(getContainer(), blobStore.blobBuilder(iDir + "j" + j + "/k" + k).payload(new byte[0]).build()); + } + } + blobStore.putBlob(getContainer(), blobStore.blobBuilder(iDir + "extra").payload(new byte[0]).build()); + LOGGER.log(Level.INFO, "added 101 blobs to {0}", iDir); + } + httpLogging.record(InvokeHttpMethod.class, Level.FINE); + httpLogging.capture(1000); + // Default list page size for S3 is 1000 blobs; we have 1010 plus the two created for all tests, so should hit a second page. + assertThat(subdir.list("sprawling/**/k3", null, true), iterableWithSize(100)); + assertEquals("calls GetBucketLocation then ListBucket, advance to …/sprawling/i9/j8/k8, ListBucket again", 3, httpLogging.getRecords().size()); + } + @Test public void open() throws Exception { try (InputStream is = subdir.open()) {