leveldb::DestroyDB will now delete empty directories.

Env's that filtered out dot files ("." and "..") would return an
empty vector of children causing DestroyDB to do nothing. This fixes
#215

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=172501335

db/db_impl.cc

@@ -1537,15 +1537,15 @@ Snapshot::~Snapshot() {
 Status DestroyDB(const std::string& dbname, const Options& options) {
   Env* env = options.env;
   std::vector<std::string> filenames;
-  // Ignore error in case directory does not exist
-  env->GetChildren(dbname, &filenames);
-  if (filenames.empty()) {
+  Status result = env->GetChildren(dbname, &filenames);
+  if (!result.ok()) {
+    // Ignore error in case directory does not exist
     return Status::OK();
   }
 
   FileLock* lock;
   const std::string lockname = LockFileName(dbname);
-  Status result = env->LockFile(lockname, &lock);
+  result = env->LockFile(lockname, &lock);
   if (result.ok()) {
     uint64_t number;
     FileType type;

db/db_test.cc

@@ -61,6 +61,36 @@ void DelayMilliseconds(int millis) {
 }
 }
 
+// Test Env to override default Env behavior for testing.
+class TestEnv : public EnvWrapper {
+ public:
+  explicit TestEnv(Env* base) : EnvWrapper(base), ignore_dot_files_(false) {}
+
+  void SetIgnoreDotFiles(bool ignored) { ignore_dot_files_ = ignored; }
+
+  Status GetChildren(const std::string& dir,
+                     std::vector<std::string>* result) override {
+    Status s = target()->GetChildren(dir, result);
+    if (!s.ok() || !ignore_dot_files_) {
+      return s;
+    }
+
+    std::vector<std::string>::iterator it = result->begin();
+    while (it != result->end()) {
+      if ((*it == ".") || (*it == "..")) {
+        it = result->erase(it);
+      } else {
+        ++it;
+      }
+    }
+
+    return s;
+  }
+
+ private:
+  bool ignore_dot_files_;
+};
+
 // Special Env used to delay background operations
 class SpecialEnv : public EnvWrapper {
  public:
@@ -1561,6 +1591,58 @@ TEST(DBTest, DBOpen_Options) {
   db = NULL;
 }
 
+TEST(DBTest, DestroyEmptyDir) {
+  std::string dbname = test::TmpDir() + "/db_empty_dir";
+  TestEnv env(Env::Default());
+  env.DeleteDir(dbname);
+  ASSERT_TRUE(!env.FileExists(dbname));
+
+  Options opts;
+  opts.env = &env;
+
+  ASSERT_OK(env.CreateDir(dbname));
+  ASSERT_TRUE(env.FileExists(dbname));
+  std::vector<std::string> children;
+  ASSERT_OK(env.GetChildren(dbname, &children));
+  // The POSIX env does not filter out '.' and '..' special files.
+  ASSERT_EQ(2, children.size());
+  ASSERT_OK(DestroyDB(dbname, opts));
+  ASSERT_TRUE(!env.FileExists(dbname));
+
+  // Should also be destroyed if Env is filtering out dot files.
+  env.SetIgnoreDotFiles(true);
+  ASSERT_OK(env.CreateDir(dbname));
+  ASSERT_TRUE(env.FileExists(dbname));
+  ASSERT_OK(env.GetChildren(dbname, &children));
+  ASSERT_EQ(0, children.size());
+  ASSERT_OK(DestroyDB(dbname, opts));
+  ASSERT_TRUE(!env.FileExists(dbname));
+}
+
+TEST(DBTest, DestroyOpenDB) {
+  std::string dbname = test::TmpDir() + "/open_db_dir";
+  env_->DeleteDir(dbname);
+  ASSERT_TRUE(!env_->FileExists(dbname));
+
+  Options opts;
+  opts.create_if_missing = true;
+  DB* db = NULL;
+  ASSERT_OK(DB::Open(opts, dbname, &db));
+  ASSERT_TRUE(db != NULL);
+
+  // Must fail to destroy an open db.
+  ASSERT_TRUE(env_->FileExists(dbname));
+  ASSERT_TRUE(!DestroyDB(dbname, Options()).ok());
+  ASSERT_TRUE(env_->FileExists(dbname));
+
+  delete db;
+  db = NULL;
+
+  // Should succeed destroying a closed db.
+  ASSERT_OK(DestroyDB(dbname, Options()));
+  ASSERT_TRUE(!env_->FileExists(dbname));
+}
+
 TEST(DBTest, Locking) {
   DB* db2 = NULL;
   Status s = DB::Open(CurrentOptions(), dbname_, &db2);

include/leveldb/db.h

@@ -151,6 +151,9 @@ class LEVELDB_EXPORT DB {
 
 // Destroy the contents of the specified database.
 // Be very careful using this method.
+//
+// Note: For backwards compatibility, if DestroyDB is unable to list the
+// database files, Status::OK() will still be returned masking this failure.
 LEVELDB_EXPORT Status DestroyDB(const std::string& name,
                                 const Options& options);