Bug 1289006 - Add code to assert potential crashing case never happens & tests. r=grisha

I expect the crashes occurred because one of the following:
  * the store already existed as a file
  * the store was a directory that did not have the appropriate permissions

In the telemetry code, none of the cases above should happen so I assert that
they never do.

If the crashes did occur for these reasons, the user will unfortunately
continue to crash but at least we'll know where our assumptions are going wrong.

I originally intended to write a regression test for listFiles returning null
but it requires the application code to be modified in non-trivial ways (e.g.
accessor methods we might forget to use) so I decided against it.

MozReview-Commit-ID: 9V9H84ehbdO

--HG--
extra : rebase_source : 8290e515c9010bef639e92d1b0420bebe5c7d61c
This commit is contained in:
Michael Comella 2016-07-25 14:24:03 -07:00
Родитель bd05d8b08b
Коммит 7e2617e508
2 изменённых файлов: 39 добавлений и 0 удалений

Просмотреть файл

@ -78,9 +78,20 @@ public class TelemetryJSONFilePingStore extends TelemetryPingStore {
@WorkerThread // Writes to disk
public TelemetryJSONFilePingStore(final File storeDir, final String profileName) {
super(profileName);
if (storeDir.exists() && !storeDir.isDirectory()) {
// An alternative is to create a new directory, but we wouldn't
// be able to access it later so it's better to throw.
throw new IllegalStateException("Store dir unexpectedly exists & is not a directory - cannot continue");
}
this.storeDir = storeDir;
this.storeDir.mkdirs();
uuidFilenameFilter = new FilenameRegexFilter(UUIDUtil.UUID_PATTERN);
if (!this.storeDir.canRead() || !this.storeDir.canWrite() || !this.storeDir.canExecute()) {
throw new IllegalStateException("Cannot read, write, or execute store dir: " +
this.storeDir.canRead() + " " + this.storeDir.canWrite() + " " + this.storeDir.canExecute());
}
}
@VisibleForTesting File getPingFile(final String docID) {

Просмотреть файл

@ -19,6 +19,7 @@ import org.mozilla.gecko.util.FileUtils;
import java.io.File;
import java.io.FileOutputStream;
import java.io.FilenameFilter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
@ -70,6 +71,33 @@ public class TestTelemetryJSONFilePingStore {
assertEquals("Temp dir contains one dir (the store dir)", 1, tempDir.getRoot().list().length);
}
@Test(expected = IllegalStateException.class)
public void testConstructorStoreAlreadyExistsAsNonDirectory() throws Exception {
final File file = tempDir.newFile();
new TelemetryJSONFilePingStore(file, "profileName"); // expected to throw.
}
@Test(expected = IllegalStateException.class)
public void testConstructorDirIsNotReadable() throws Exception {
final File dir = tempDir.newFolder();
dir.setReadable(false);
new TelemetryJSONFilePingStore(dir, "profileName"); // expected to throw.
}
@Test(expected = IllegalStateException.class)
public void testConstructorDirIsNotWritable() throws Exception {
final File dir = tempDir.newFolder();
dir.setWritable(false);
new TelemetryJSONFilePingStore(dir, "profileName"); // expected to throw.
}
@Test(expected = IllegalStateException.class)
public void testConstructorDirIsNotExecutable() throws Exception {
final File dir = tempDir.newFolder();
dir.setExecutable(false);
new TelemetryJSONFilePingStore(dir, "profileName"); // expected to throw.
}
@Test
public void testStorePingStoresCorrectData() throws Exception {
assertStoreFileCount(0);