Skip to content

Commit

Permalink
Avoid "ZipException: error in opening zip file" thrown when artifacts…
Browse files Browse the repository at this point in the history 
… read while being written
  • Loading branch information
@olivergondza
olivergondza committed Feb 21, 2015
1 parent 8dee98b commit 4720879
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 16 deletions.
41 changes: 25 additions & 16 deletions src/main/java/org/jenkinsci/plugins/compress_artifacts/ZipStorage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,17 @@ static VirtualFile root(File archive) {
return new ZipStorage(archive, "");
}

// TODO support updating entries
static void archive(File archive, FilePath workspace, Launcher launcher, BuildListener listener, Map<String,String> artifacts) throws IOException, InterruptedException {
// TODO support updating entries
OutputStream os = new FileOutputStream(archive);
// Use temporary file for writing, rename when done
File writingArchive = new File(archive.getAbsolutePath() + ".writing");
OutputStream os = new FileOutputStream(writingArchive);
try {
workspace.zip(os, new FilePath.ExplicitlySpecifiedDirScanner(artifacts));
} finally {
os.close();
}
writingArchive.renameTo(archive);
}

static boolean delete(File archive) throws IOException, InterruptedException {
Expand Down Expand Up @@ -115,7 +118,7 @@ private boolean looksLikeDir() {
}

@Override public boolean isDirectory() throws IOException {
if (!looksLikeDir()) {
if (!looksLikeDir() || !archive.exists()) {
return false;
}
ZipFile zf = new ZipFile(archive);
Expand All @@ -135,7 +138,7 @@ private boolean looksLikeDir() {
}

@Override public boolean isFile() throws IOException {
if (looksLikeDir()) {
if (looksLikeDir() || !archive.exists()) {
return false;
}
ZipFile zf = new ZipFile(archive);
Expand All @@ -147,6 +150,8 @@ private boolean looksLikeDir() {
}

@Override public boolean exists() throws IOException {
if (!archive.exists()) return false;

ZipFile zf = new ZipFile(archive);
try {
Enumeration<? extends ZipEntry> entries = zf.entries();
Expand All @@ -164,7 +169,7 @@ private boolean looksLikeDir() {
}

@Override public VirtualFile[] list() throws IOException {
if (!looksLikeDir()) {
if (!looksLikeDir() || !archive.exists()) {
return new VirtualFile[0];
}
ZipFile zf = new ZipFile(archive);
Expand All @@ -185,16 +190,16 @@ private boolean looksLikeDir() {
}

@Override public String[] list(String glob) throws IOException {
if (! looksLikeDir()) {
throw new IOException("Not a directory");
}
if (!looksLikeDir() || !archive.exists()) {
return new String[0];
}

// canonical implementation treats null glob the same as empty string
if (glob==null) {
glob="";
}

// canonical implementation treats null glob the same as empty string
if (glob==null) {
glob="";
}

ZipFile zf = new ZipFile(archive);
ZipFile zf = new ZipFile(archive);
try {
Set<String> files = new HashSet<String>();
Enumeration<? extends ZipEntry> entries = zf.entries();
Expand All @@ -211,7 +216,6 @@ private boolean looksLikeDir() {
} finally {
zf.close();
}

}

@Override public VirtualFile child(String name) {
Expand All @@ -227,6 +231,8 @@ private boolean looksLikeDir() {
}

@Override public long length() throws IOException {
if (!archive.exists()) return 0;

ZipFile zf = new ZipFile(archive);
try {
ZipEntry entry = zf.getEntry(path);
Expand All @@ -237,6 +243,8 @@ private boolean looksLikeDir() {
}

@Override public long lastModified() throws IOException {
if (!archive.exists()) return 0;

ZipFile zf = new ZipFile(archive);
try {
ZipEntry entry = zf.getEntry(path);
Expand All @@ -251,6 +259,8 @@ private boolean looksLikeDir() {
}

@Override public InputStream open() throws IOException {
if (!archive.exists()) throw new FileNotFoundException(path + " (No such file or directory)");

if (looksLikeDir()) {
// That is what java.io.FileInputStream.open throws
throw new FileNotFoundException(this + " (Is a directory)");
Expand All @@ -267,5 +277,4 @@ private boolean looksLikeDir() {
}
};
}

}
100 changes: 100 additions & 0 deletions src/test/java/org/jenkinsci/plugins/compress_artifacts/ZipStorageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,13 @@
import java.io.IOException;
import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;

import jenkins.util.VirtualFile;

Expand Down Expand Up @@ -207,6 +212,101 @@ private void doRead(VirtualFile vf) throws Exception {
assertEquals(0, vf.child("thre_is_none").length());
}

@Test
public void avoidZipExceptionWhileWriting() throws Exception {
FileUtils.writeStringToFile(new File(content, "file"), "content");

final Entry<String, String> validArtifact = Collections.singletonMap("file", "file").entrySet().iterator().next();

// Simulate archiving that takes forever serving valid artifact and then block forever on the next.
final Map<String,String> artifacts = new HashMap<String,String>() {
@Override
public Set<Map.Entry<String, String>> entrySet() {
return new HashSet<Map.Entry<String, String>>() {
@Override
public Iterator<Map.Entry<String, String>> iterator() {
return new Iterator<Map.Entry<String, String>>() {
private boolean block = false;

public boolean hasNext() {
return true;
}

public Map.Entry<String, String> next() {
if (!block) {
block = true;
return validArtifact;
}

synchronized (this) {
try {
this.wait(); // Block forever
throw new AssertionError();
} catch (InterruptedException ex) {
// Expected at cleanup time
}
}

return validArtifact;
}

public void remove() {
throw new UnsupportedOperationException();
}
};
}
};
}
};

// start archiving
Thread compressor = new Thread("compressing-thread") {
@Override
public void run() {
try {
archive(artifacts);
} catch (Exception ex) {
throw new Error(ex);
}
}
};

compressor.start();
try {
Thread.sleep(1000);

assertTrue(compressor.isAlive());

assertArrayEquals(new String[0], zs.list());
assertArrayEquals(new VirtualFile[0], zs.list("*"));
} finally {
compressor.stop();
}
}

@Test // This can happen when it was not yet (fully) written or it was deleted
public void supporMissingArchiveFile() throws Exception {
assertArrayEquals(new String[0], zs.list());
assertArrayEquals(new VirtualFile[0], zs.list("*"));
assertFalse(zs.exists());
assertFalse(zs.isFile());
assertFalse(zs.isDirectory());

VirtualFile child = zs.child("child");
assertFalse(child.exists());
assertFalse(child.isFile());
assertFalse(child.isDirectory());
assertEquals(0, child.length());
assertEquals(0, child.lastModified());

try {
child.open();
fail();
} catch (IOException ex) {
assertTrue(ex instanceof FileNotFoundException);
}
}

private void archive(Map<String, String> artifacts) throws Exception {
BuildListener l = new StreamBuildListener(System.out, Charset.defaultCharset());
ZipStorage.archive(archive, new FilePath(content), new Launcher.LocalLauncher(l), l, artifacts);
Expand Down

0 comments on commit 4720879

Please sign in to comment.