Move DirectoryReader::ReadEntries to FileRef::ReadDirectoryEntries

ppapi/tests/test_file_ref.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ppapi/tests/test_file_ref.h"
 
 #include <stdio.h>
 #include <vector>
 
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/ppb_file_io.h"
 #include "ppapi/c/dev/ppb_testing_dev.h"
-#include "ppapi/cpp/dev/directory_entry_dev.h"
-#include "ppapi/cpp/dev/directory_reader_dev.h"
+#include "ppapi/cpp/directory_entry.h"
 #include "ppapi/cpp/file_io.h"
 #include "ppapi/cpp/file_ref.h"
 #include "ppapi/cpp/file_system.h"
 #include "ppapi/cpp/instance.h"
 #include "ppapi/cpp/module.h"
 #include "ppapi/cpp/url_loader.h"
 #include "ppapi/cpp/url_request_info.h"
 #include "ppapi/cpp/url_response_info.h"
 #include "ppapi/tests/test_utils.h"
 #include "ppapi/tests/testing_instance.h"
 
 REGISTER_TEST_CASE(FileRef);
 
 namespace {
 
 const char* kPersFileName = "persistent";
 const char* kTempFileName = "temporary";
 const char* kParentPath = "/foo/bar";
 const char* kPersFilePath = "/foo/bar/persistent";
 const char* kTempFilePath = "/foo/bar/temporary";
 const char* kTerribleName = "!@#$%^&*()-_=+{}[] ;:'\"|`~\t\n\r\b?";
 
+typedef std::vector<pp::DirectoryEntry> DirEntries;
+
 std::string ReportMismatch(const std::string& method_name,
                            const std::string& returned_result,
                            const std::string& expected_result) {
   return method_name + " returned '" + returned_result + "'; '" +
       expected_result + "' expected.";
 }
 
+std::string IntegerToString(int value) {
+  char result[12];
+  sprintf(result, "%d", value);

[dmichael (off chromium), 2013/05/02 19:43:54]

nit: please use snprintf (or just delete this function; see below)

[hamaji, 2013/05/02 20:14:49]

Removed

+  return result;
+}
+
 }  // namespace
 
 bool TestFileRef::Init() {
   return CheckTestingInterface() && EnsureRunningOverHTTP();
 }
 
 std::string TestFileRef::MakeExternalFileRef(pp::FileRef* file_ref_ext) {
   pp::URLRequestInfo request(instance_);
   request.SetURL("test_url_loader_data/hello.txt");
   request.SetStreamToFile(true);
 
   TestCompletionCallback callback(instance_->pp_instance(), callback_type());
 
   pp::URLLoader loader(instance_);
   callback.WaitForResult(loader.Open(request, callback.GetCallback()));
   CHECK_CALLBACK_BEHAVIOR(callback);
   ASSERT_EQ(PP_OK, callback.result());
 
   pp::URLResponseInfo response_info(loader.GetResponseInfo());
   ASSERT_FALSE(response_info.is_null());
   ASSERT_EQ(200, response_info.GetStatusCode());
 
   *file_ref_ext = pp::FileRef(response_info.GetBodyAsFileRef());
   ASSERT_EQ(PP_FILESYSTEMTYPE_EXTERNAL, file_ref_ext->GetFileSystemType());
   PASS();
 }
 
+int32_t TestFileRef::DeleteDirectoryRecursively(pp::FileRef* dir) {
+  if (!dir)
+    return PP_ERROR_BADARGUMENT;
+
+  TestCompletionCallback callback(instance_->pp_instance(), callback_type());
+  TestCompletionCallbackWithOutput<DirEntries> output_callback(
+      instance_->pp_instance(), callback_type());
+
+  output_callback.WaitForResult(
+      dir->ReadEntries(output_callback.GetCallback()));
+  int32_t rv = output_callback.result();
+  if (rv != PP_OK && rv != PP_ERROR_FILENOTFOUND)
+    return rv;
+
+  DirEntries entries = output_callback.output();
+  for (DirEntries::const_iterator it = entries.begin();
+       it != entries.end(); ++it) {

[dmichael (off chromium), 2013/05/02 19:43:54]

nit: I prefer each statement in the loop have its own line if they don't fit on
1

[hamaji, 2013/05/02 20:14:49]

Yeah, I actually have the same preference :) (as you might know, I just copied
most code from test_directory_reader.cc)

+    pp::FileRef file_ref = it->file_ref();
+    if (it->file_type() == PP_FILETYPE_DIRECTORY) {
+      rv = DeleteDirectoryRecursively(&file_ref);
+      if (rv != PP_OK && rv != PP_ERROR_FILENOTFOUND)
+        return rv;
+    } else {
+      callback.WaitForResult(file_ref.Delete(callback.GetCallback()));
+      rv = callback.result();
+      if (rv != PP_OK && rv != PP_ERROR_FILENOTFOUND)
+        return rv;
+    }
+  }
+  callback.WaitForResult(dir->Delete(callback.GetCallback()));
+  return callback.result();
+}
+
 void TestFileRef::RunTests(const std::string& filter) {
   RUN_CALLBACK_TEST(TestFileRef, Create, filter);
   RUN_CALLBACK_TEST(TestFileRef, GetFileSystemType, filter);
   RUN_CALLBACK_TEST(TestFileRef, GetName, filter);
   RUN_CALLBACK_TEST(TestFileRef, GetPath, filter);
   RUN_CALLBACK_TEST(TestFileRef, GetParent, filter);
   RUN_CALLBACK_TEST(TestFileRef, MakeDirectory, filter);
   RUN_CALLBACK_TEST(TestFileRef, QueryAndTouchFile, filter);
   RUN_CALLBACK_TEST(TestFileRef, DeleteFileAndDirectory, filter);
   RUN_CALLBACK_TEST(TestFileRef, RenameFileAndDirectory, filter);
   RUN_CALLBACK_TEST(TestFileRef, Query, filter);
   RUN_CALLBACK_TEST(TestFileRef, FileNameEscaping, filter);
+  // FileRef::ReadEntries is in-process only.
+  if (testing_interface_->IsOutOfProcess())
+    RUN_CALLBACK_TEST(TestFileRef, ReadEntries, filter);
 }
 
 std::string TestFileRef::TestCreate() {
   std::vector<std::string> invalid_paths;
   invalid_paths.push_back("invalid_path");  // no '/' at the first character
   invalid_paths.push_back(std::string());   // empty path
   // The following are directory traversal checks
   invalid_paths.push_back("..");
   invalid_paths.push_back("/../invalid_path");
   invalid_paths.push_back("/../../invalid_path");
@@ skipping 488 matching lines @@
 
   // Create the file with the terrible name.
   std::string full_file_path = test_dir_path + "/" + kTerribleName;
   pp::FileRef file_ref(file_system, full_file_path.c_str());
   pp::FileIO file_io(instance_);
   callback.WaitForResult(
       file_io.Open(file_ref, PP_FILEOPENFLAG_CREATE, callback.GetCallback()));
   CHECK_CALLBACK_BEHAVIOR(callback);
   ASSERT_EQ(PP_OK, callback.result());
 
-  // DirectoryReader only works out-of-process.
+  // FileRef::ReadEntries only works out-of-process.
   if (testing_interface_->IsOutOfProcess()) {
-    TestCompletionCallbackWithOutput< std::vector<pp::DirectoryEntry_Dev> >
+    TestCompletionCallbackWithOutput<DirEntries>
         output_callback(instance_->pp_instance(), callback_type());
-    pp::DirectoryReader_Dev directory_reader(test_dir_ref);
 
     output_callback.WaitForResult(
-        directory_reader.ReadEntries(output_callback.GetCallback()));
+        test_dir_ref.ReadEntries(output_callback.GetCallback()));
     CHECK_CALLBACK_BEHAVIOR(output_callback);
     ASSERT_EQ(PP_OK, output_callback.result());
 
-    std::vector<pp::DirectoryEntry_Dev> entries = output_callback.output();
+    DirEntries entries = output_callback.output();
     ASSERT_EQ(1, entries.size());
     ASSERT_EQ(kTerribleName, entries.front().file_ref().GetName().AsString());
   }
 
   PASS();
 }
+
+std::string TestFileRef::TestReadEntries() {
+  TestCompletionCallback callback(instance_->pp_instance(), callback_type());
+  pp::FileSystem file_system(
+      instance_, PP_FILESYSTEMTYPE_LOCALTEMPORARY);
+  callback.WaitForResult(file_system.Open(1024, callback.GetCallback()));
+  CHECK_CALLBACK_BEHAVIOR(callback);
+  ASSERT_EQ(PP_OK, callback.result());
+
+  // Setup testing directories and files.
+  const char* test_dir_name = "/test_get_next_file";
+  const char* file_prefix = "file_";
+  const char* dir_prefix = "dir_";
+
+  pp::FileRef test_dir(file_system, test_dir_name);
+  int32_t rv = DeleteDirectoryRecursively(&test_dir);
+  if (rv != PP_OK && rv != PP_ERROR_FILENOTFOUND)
+    return ReportError("DeleteDirectoryRecursively", rv);

[dmichael (off chromium), 2013/05/02 19:43:54]

nit: I'd prefer just:
ASSERT_TRUE(rv == PP_OK || rv == PP_ERROR_FILENOTFOUND);

[hamaji, 2013/05/02 20:14:49]

Done.

+
+  callback.WaitForResult(test_dir.MakeDirectory(callback.GetCallback()));
+  CHECK_CALLBACK_BEHAVIOR(callback);
+  ASSERT_EQ(PP_OK, callback.result());
+
+  std::set<std::string> expected_file_names;
+  for (int i = 1; i < 4; ++i) {
+    char buffer[40];
+    sprintf(buffer, "%s/%s%d", test_dir_name, file_prefix, i);

[dmichael (off chromium), 2013/05/02 19:43:54]

snprintf

[hamaji, 2013/05/02 20:14:49]

Done.

+    pp::FileRef file_ref(file_system, buffer);
+
+    pp::FileIO file_io(instance_);
+    callback.WaitForResult(
+        file_io.Open(file_ref, PP_FILEOPENFLAG_CREATE, callback.GetCallback()));
+    CHECK_CALLBACK_BEHAVIOR(callback);
+    ASSERT_EQ(PP_OK, callback.result());
+
+    expected_file_names.insert(buffer);
+  }
+
+  std::set<std::string> expected_dir_names;
+  for (int i = 1; i < 4; ++i) {

[dmichael (off chromium), 2013/05/02 19:43:54]

suggestion: May make sense to use a constant like:
const int kNumDirectories = 3;
And loop while "i <= kNumDirectories" for readability and to keep the lopos in
sync

[hamaji, 2013/05/02 20:14:49]

It seems this loop creates directories but the loop above creates files. That
said, I agree adding kNum* would improve readability. Added kNumFiles and
kNumDirectories.

+    char buffer[40];
+    sprintf(buffer, "%s/%s%d", test_dir_name, dir_prefix, i);

[dmichael (off chromium), 2013/05/02 19:43:54]

snprintf

[hamaji, 2013/05/02 20:14:49]

Done.

+    pp::FileRef file_ref(file_system, buffer);
+
+    callback.WaitForResult(file_ref.MakeDirectory(callback.GetCallback()));
+    CHECK_CALLBACK_BEHAVIOR(callback);
+    ASSERT_EQ(PP_OK, callback.result());
+
+    expected_dir_names.insert(buffer);
+  }
+
+  // Test that |ReadEntries()| is able to fetch all directories and files that
+  // we created.
+  {
+    TestCompletionCallbackWithOutput<DirEntries> output_callback(
+        instance_->pp_instance(), callback_type());
+
+    output_callback.WaitForResult(
+        test_dir.ReadEntries(output_callback.GetCallback()));
+    CHECK_CALLBACK_BEHAVIOR(output_callback);
+    ASSERT_EQ(PP_OK, output_callback.result());
+
+    DirEntries entries = output_callback.output();
+    size_t sum = expected_file_names.size() + expected_dir_names.size();
+    if (entries.size() != sum)
+      return "Expected " + IntegerToString(sum) + " entries, got " +
+          IntegerToString(entries.size());

[dmichael (off chromium), 2013/05/02 19:43:54]

How about just ASSERT_EQ(entries.size(), sum);
Then you don't even need IntegerToString.

[hamaji, 2013/05/02 20:14:49]

Done.

+
+    for (DirEntries::const_iterator it = entries.begin();
+         it != entries.end(); ++it) {
+      pp::FileRef file_ref = it->file_ref();
+      std::string file_path = file_ref.GetPath().AsString();
+      std::set<std::string>::iterator found =
+          expected_file_names.find(file_path);
+      if (found != expected_file_names.end()) {
+        if (it->file_type() != PP_FILETYPE_REGULAR)
+          return file_path + " should have been a regular file.";
+        expected_file_names.erase(found);
+      } else {
+        found = expected_dir_names.find(file_path);
+        if (found == expected_dir_names.end())
+          return "Unexpected file path: " + file_path;
+        if (it->file_type() != PP_FILETYPE_DIRECTORY)
+          return file_path + " should have been a directory.";
+        expected_dir_names.erase(found);
+      }
+    }
+    if (!expected_file_names.empty() || !expected_dir_names.empty())
+      return "Expected more file paths.";

[dmichael (off chromium), 2013/05/02 19:43:54]

ASSERT_TRUE(expected_file_names.empty());
ASSERT_TRUE(expected_dir_names.empty());

[hamaji, 2013/05/02 20:14:49]

Done.

+  }
+
+  // Test cancellation of asynchronous |ReadEntries()|.
+  TestCompletionCallbackWithOutput<DirEntries> output_callback(
+      instance_->pp_instance(), callback_type());
+  {
+    rv = pp::FileRef(file_system, test_dir_name)
+        .ReadEntries(output_callback.GetCallback());
+  }
+  output_callback.WaitForAbortResult(rv);
+  CHECK_CALLBACK_BEHAVIOR(output_callback);
+
+
+  PASS();
+}