Skip to content

Commit 56d55a5

Browse files
jean-philippe-martingarrettjonesgoogle
jean-philippe-martin
authored and
garrettjonesgoogle
committed
Allow deletion of empty folders (#1630)
(even though we normally don't allow operations on folders since they are not represented). This helps with NIO-using code that must also work with filesystems that *do* represent folders.
1 parent e7f2a14 commit 56d55a5

4 files changed

Lines changed: 172 additions & 2 deletions

File tree

‎google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import java.nio.file.DirectoryStream.Filter;
4949
import java.nio.file.FileAlreadyExistsException;
5050
import java.nio.file.FileStore;
51+
import java.nio.file.Files;
5152
import java.nio.file.LinkOption;
5253
import java.nio.file.NoSuchFileException;
5354
import java.nio.file.OpenOption;
@@ -351,6 +352,15 @@ public boolean deleteIfExists(Path path) throws IOException {
351352
initStorage();
352353
CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path);
353354
if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories()) {
355+
// if the "folder" is empty then we're fine, otherwise complain
356+
// that we cannot act on folders.
357+
try (DirectoryStream<Path> paths = Files.newDirectoryStream(path)) {
358+
if (!paths.iterator().hasNext()) {
359+
// "folder" isn't actually there in the first place, so: success!
360+
// (we must return true so delete doesn't freak out)
361+
return true;
362+
}
363+
}
354364
throw new CloudStoragePseudoDirectoryException(cloudPath);
355365
}
356366
return storage.delete(cloudPath.getBlobId());

‎google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,6 @@ public void testDelete_dotDirNotNormalized_throwsIae() throws IOException {
413413

414414
@Test
415415
public void testDelete_trailingSlash() throws IOException {
416-
thrown.expect(CloudStoragePseudoDirectoryException.class);
417416
Files.delete(Paths.get(URI.create("gs://love/passion/")));
418417
}
419418

@@ -442,7 +441,6 @@ public void testDeleteIfExists() throws IOException {
442441

443442
@Test
444443
public void testDeleteIfExists_trailingSlash() throws IOException {
445-
thrown.expect(CloudStoragePseudoDirectoryException.class);
446444
Files.deleteIfExists(Paths.get(URI.create("gs://love/passion/")));
447445
}
448446

‎google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@
2424
import com.google.common.testing.NullPointerTester;
2525

2626
import org.junit.Before;
27+
import org.junit.Rule;
2728
import org.junit.Test;
29+
import org.junit.rules.ExpectedException;
2830
import org.junit.runner.RunWith;
2931
import org.junit.runners.JUnit4;
3032

@@ -38,6 +40,9 @@
3840
import java.nio.file.Paths;
3941
import java.util.ArrayList;
4042
import java.util.List;
43+
import java.nio.file.SimpleFileVisitor;
44+
import java.nio.file.FileVisitResult;
45+
import java.nio.file.attribute.BasicFileAttributes;
4146

4247
/**
4348
* Unit tests for {@link CloudStorageFileSystem}.
@@ -55,6 +60,9 @@ public class CloudStorageFileSystemTest {
5560
+ "The Heart-ache, and the thousand Natural shocks\n"
5661
+ "That Flesh is heir to? 'Tis a consummation\n";
5762

63+
@Rule
64+
public ExpectedException thrown = ExpectedException.none();
65+
5866
@Before
5967
public void before() {
6068
CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.getOptions());
@@ -187,6 +195,117 @@ public void testMatcher() throws IOException {
187195
}
188196
}
189197

198+
@Test
199+
public void testDeleteEmptyFolder() throws IOException {
200+
try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket")) {
201+
List<Path> paths = new ArrayList<>();
202+
paths.add(fs.getPath("dir/angel"));
203+
paths.add(fs.getPath("dir/dir2/another_angel"));
204+
paths.add(fs.getPath("atroot"));
205+
for (Path path : paths) {
206+
Files.write(path, ALONE.getBytes(UTF_8));
207+
}
208+
// we can delete non-existent folders, because they are not represented on disk anyways.
209+
Files.delete(fs.getPath("ghost/"));
210+
Files.delete(fs.getPath("dir/ghost/"));
211+
Files.delete(fs.getPath("dir/dir2/ghost/"));
212+
// likewise, deleteIfExists works.
213+
Files.deleteIfExists(fs.getPath("ghost/"));
214+
Files.deleteIfExists(fs.getPath("dir/ghost/"));
215+
Files.deleteIfExists(fs.getPath("dir/dir2/ghost/"));
216+
}
217+
}
218+
219+
@Test
220+
public void testDeleteFullFolder() throws IOException {
221+
thrown.expect(CloudStoragePseudoDirectoryException.class);
222+
try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket")) {
223+
Files.write(fs.getPath("dir/angel"), ALONE.getBytes(UTF_8));
224+
// we cannot delete existing folders if they contain something
225+
Files.delete(fs.getPath("dir/"));
226+
}
227+
}
228+
229+
@Test
230+
public void testDelete() throws IOException {
231+
try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket")) {
232+
List<Path> paths = new ArrayList<>();
233+
paths.add(fs.getPath("dir/angel"));
234+
paths.add(fs.getPath("dir/dir2/another_angel"));
235+
paths.add(fs.getPath("atroot"));
236+
for (Path path : paths) {
237+
Files.write(path, ALONE.getBytes(UTF_8));
238+
}
239+
Files.delete(fs.getPath("atroot"));
240+
Files.delete(fs.getPath("dir/angel"));
241+
Files.deleteIfExists(fs.getPath("dir/dir2/another_angel"));
242+
243+
for (Path path : paths) {
244+
assertThat(Files.exists(path)).isFalse();
245+
}
246+
}
247+
}
248+
249+
@Test
250+
public void testDeleteEmptiedFolder() throws IOException {
251+
try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket")) {
252+
List<Path> paths = new ArrayList<>();
253+
paths.add(fs.getPath("dir/angel"));
254+
paths.add(fs.getPath("dir/dir2/another_angel"));
255+
for (Path path : paths) {
256+
Files.write(path, ALONE.getBytes(UTF_8));
257+
}
258+
Files.delete(fs.getPath("dir/angel"));
259+
Files.deleteIfExists(fs.getPath("dir/dir2/another_angel"));
260+
// delete folder (trailing slash is required)
261+
Path dir2 = fs.getPath("dir/dir2/");
262+
Files.deleteIfExists(dir2);
263+
Path dir = fs.getPath("dir/");
264+
Files.deleteIfExists(dir);
265+
// We can't check Files.exists on a folder (since GCS fakes folders)
266+
}
267+
}
268+
269+
@Test
270+
public void testDeleteRecursive() throws IOException {
271+
try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket")) {
272+
List<Path> paths = new ArrayList<>();
273+
paths.add(fs.getPath("atroot"));
274+
paths.add(fs.getPath("dir/angel"));
275+
paths.add(fs.getPath("dir/dir2/another_angel"));
276+
paths.add(fs.getPath("dir/dir2/angel3"));
277+
paths.add(fs.getPath("dir/dir3/cloud"));
278+
for (Path path : paths) {
279+
Files.write(path, ALONE.getBytes(UTF_8));
280+
}
281+
282+
deleteRecursive(fs.getPath("dir/"));
283+
assertThat(Files.exists(fs.getPath("dir/angel"))).isFalse();
284+
assertThat(Files.exists(fs.getPath("dir/dir3/cloud"))).isFalse();
285+
assertThat(Files.exists(fs.getPath("atroot"))).isTrue();
286+
}
287+
}
288+
289+
/**
290+
* Delete the given directory and all of its contents if non-empty.
291+
* @param directory the directory to delete
292+
* @throws IOException
293+
*/
294+
private static void deleteRecursive(Path directory) throws IOException {
295+
Files.walkFileTree(directory, new SimpleFileVisitor<Path>() {
296+
@Override
297+
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
298+
Files.delete(file);
299+
return FileVisitResult.CONTINUE;
300+
}
301+
@Override
302+
public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
303+
Files.delete(dir);
304+
return FileVisitResult.CONTINUE;
305+
}
306+
});
307+
}
308+
190309
private void assertMatches(FileSystem fs, PathMatcher matcher, String toMatch, boolean expected) {
191310
assertThat(matcher.matches(fs.getPath(toMatch).getFileName())).isEqualTo(expected);
192311
}

‎google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,13 @@
4444
import java.nio.channels.ReadableByteChannel;
4545
import java.nio.channels.SeekableByteChannel;
4646
import java.nio.file.FileSystem;
47+
import java.nio.file.FileVisitResult;
4748
import java.nio.file.Files;
4849
import java.nio.file.NoSuchFileException;
4950
import java.nio.file.Path;
51+
import java.nio.file.SimpleFileVisitor;
5052
import java.nio.file.StandardOpenOption;
53+
import java.nio.file.attribute.BasicFileAttributes;
5154
import java.util.ArrayList;
5255
import java.util.Arrays;
5356
import java.util.List;
@@ -361,6 +364,46 @@ public void testListFiles() throws IOException {
361364
}
362365
}
363366

367+
368+
@Test
369+
public void testDeleteRecursive() throws IOException {
370+
try (FileSystem fs = getTestBucket()) {
371+
List<Path> paths = new ArrayList<>();
372+
paths.add(fs.getPath("Racine"));
373+
paths.add(fs.getPath("playwrights/Moliere"));
374+
paths.add(fs.getPath("playwrights/French/Corneille"));
375+
for (Path path : paths) {
376+
Files.write(path, FILE_CONTENTS, UTF_8);
377+
}
378+
deleteRecursive(fs.getPath("playwrights/"));
379+
assertThat(Files.exists(fs.getPath("playwrights/Moliere"))).isFalse();
380+
assertThat(Files.exists(fs.getPath("playwrights/French/Corneille"))).isFalse();
381+
assertThat(Files.exists(fs.getPath("Racine"))).isTrue();
382+
Files.deleteIfExists(fs.getPath("Racine"));
383+
assertThat(Files.exists(fs.getPath("Racine"))).isFalse();
384+
}
385+
}
386+
387+
/**
388+
* Delete the given directory and all of its contents if non-empty.
389+
* @param directory the directory to delete
390+
* @throws IOException
391+
*/
392+
private static void deleteRecursive(Path directory) throws IOException {
393+
Files.walkFileTree(directory, new SimpleFileVisitor<Path>() {
394+
@Override
395+
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
396+
Files.delete(file);
397+
return FileVisitResult.CONTINUE;
398+
}
399+
@Override
400+
public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
401+
Files.delete(dir);
402+
return FileVisitResult.CONTINUE;
403+
}
404+
});
405+
}
406+
364407
private int readFully(ReadableByteChannel chan, byte[] outputBuf) throws IOException {
365408
ByteBuffer buf = ByteBuffer.wrap(outputBuf);
366409
int sofar = 0;

0 commit comments

Comments
 (0)