[alohalytics] More review fixes.

9 files changed, 42 insertions, 29 deletions

diff --git a/3party/Alohalytics/alohalytics.pro b/3party/Alohalytics/alohalytics.pro
index 85bd319f95..db52276e0f 100644
--- a/3party/Alohalytics/alohalytics.pro
+++ b/3party/Alohalytics/alohalytics.pro
@@ -29,6 +29,10 @@ macx-*|linux-* {
   SOURCES += src/posix/file_manager_posix_impl.cc
 }
 
+win* {
+  SOURCES += src/windows/file_manager_windows_impl.cc
+}
+
 linux-* {
   SOURCES += src/posix/http_client_curl.cc
 }
diff --git a/3party/Alohalytics/src/alohalytics.h b/3party/Alohalytics/src/alohalytics.h
index 4e9b3bcadd..1d13b0a5e3 100644
--- a/3party/Alohalytics/src/alohalytics.h
+++ b/3party/Alohalytics/src/alohalytics.h
@@ -48,7 +48,6 @@ class Stats final {
   // Use alohalytics::Stats::Instance() to access statistics engine.
   Stats();
 
-  // static bool UploadBuffer(const std::string & url, std::string && buffer, bool debug_mode);
   // Should return false on upload error.
   bool UploadFileImpl(bool file_name_in_content, const std::string & content);
 
diff --git a/3party/Alohalytics/src/android/java/org/alohalytics/MainApplication.java b/3party/Alohalytics/src/android/java/org/alohalytics/MainApplication.java
index 9df1de4df3..f55c05135b 100644
--- a/3party/Alohalytics/src/android/java/org/alohalytics/MainApplication.java
+++ b/3party/Alohalytics/src/android/java/org/alohalytics/MainApplication.java
@@ -1,7 +1,7 @@
 /*******************************************************************************
  The MIT License (MIT)
 
- Copyright (c) 2014 Alexander Zolotarev <me@alex.bio> from Minsk, Belarus
+ Copyright (c) 2015 Alexander Zolotarev <me@alex.bio> from Minsk, Belarus
 
  Permission is hereby granted, free of charge, to any person obtaining a copy
  of this software and associated documentation files (the "Software"), to deal
diff --git a/3party/Alohalytics/src/apple/alohalytics_objc.mm b/3party/Alohalytics/src/apple/alohalytics_objc.mm
index fe93c577da..7a847db190 100644
--- a/3party/Alohalytics/src/apple/alohalytics_objc.mm
+++ b/3party/Alohalytics/src/apple/alohalytics_objc.mm
@@ -289,6 +289,8 @@ static void OnUploadFinished(alohalytics::ProcessingResult result) {
 
 // Quick check if device has any active connection.
 // Does not guarantee actual reachability of any host.
+// Inspired by Apple's Reachability example:
+// https://developer.apple.com/library/ios/samplecode/Reachability/Introduction/Intro.html
 bool IsConnectionActive() {
   struct sockaddr_in zero;
   bzero(&zero, sizeof(zero));
diff --git a/3party/Alohalytics/src/cpp/alohalytics.cc b/3party/Alohalytics/src/cpp/alohalytics.cc
index e436b4a9ca..97004aff79 100644
--- a/3party/Alohalytics/src/cpp/alohalytics.cc
+++ b/3party/Alohalytics/src/cpp/alohalytics.cc
@@ -77,17 +77,17 @@ void Stats::GzipAndArchiveFileInTheQueue(const std::string & in_file, const std:
       std::ifstream fi;
       fi.exceptions(std::ifstream::failbit | std::ifstream::badbit);
       fi.open(in_file, std::ifstream::in | std::ifstream::binary);
-      const size_t id_size = unique_client_id_event_.size();
+      const size_t data_offset = unique_client_id_event_.size();
       const int64_t file_size = FileManager::GetFileSize(in_file);
-      buffer.resize(id_size + static_cast<std::string::size_type>(file_size));
-      fi.read(&buffer[id_size], static_cast<std::streamsize>(file_size));
+      buffer.resize(data_offset + static_cast<std::string::size_type>(file_size));
+      fi.read(&buffer[data_offset], static_cast<std::streamsize>(file_size));
     }
     {
       std::ofstream fo;
       fo.exceptions(std::ifstream::failbit | std::ifstream::badbit);
       fo.open(out_archive, std::ofstream::out | std::ofstream::binary | std::ofstream::trunc);
       const std::string gzipped_buffer = Gzip(buffer);
-      buffer.resize(0);
+      std::string().swap(buffer); // Free memory.
       fo.write(gzipped_buffer.data(), gzipped_buffer.size());
     }
   } catch (const std::exception & ex) {
diff --git a/3party/Alohalytics/src/file_manager.h b/3party/Alohalytics/src/file_manager.h
index 7240033a2e..9e635eb16e 100644
--- a/3party/Alohalytics/src/file_manager.h
+++ b/3party/Alohalytics/src/file_manager.h
@@ -22,8 +22,8 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 SOFTWARE.
 *******************************************************************************/
 
-#ifndef FILE_MANAGER_HPP
-#define FILE_MANAGER_HPP
+#ifndef FILE_MANAGER_H
+#define FILE_MANAGER_H
 
 #include <string>
 #include <functional>
@@ -52,7 +52,7 @@ struct FileManager {
     if (file_path.empty()) {
       return std::string();
     }
-    std::string::size_type slash = file_path.find_last_of(kDirectorySeparator);
+    const std::string::size_type slash = file_path.find_last_of(kDirectorySeparator);
     if (slash == std::string::npos) {
       return std::string(".");
     }
@@ -86,9 +86,10 @@ struct FileManager {
   static void ForEachFileInDir(std::string directory, std::function<bool(const std::string & full_path)> lambda);
 
   // Returns negative value on error and if full_path_to_file is a directory.
+  // TODO(AlexZ): Should consider approach with exceptions and uint64_t return type.
   static int64_t GetFileSize(const std::string & full_path_to_file);
 };
 
 }  // namespace alohalytics
 
-#endif  // FILE_MANAGER_HPP
+#endif  // FILE_MANAGER_H
diff --git a/3party/Alohalytics/src/messages_queue.h b/3party/Alohalytics/src/messages_queue.h
index 9c434aa4e9..4c59b55185 100644
--- a/3party/Alohalytics/src/messages_queue.h
+++ b/3party/Alohalytics/src/messages_queue.h
@@ -22,8 +22,8 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 SOFTWARE.
 *******************************************************************************/
 
-#ifndef MESSAGES_QUEUE_HPP
-#define MESSAGES_QUEUE_HPP
+#ifndef MESSAGES_QUEUE_H
+#define MESSAGES_QUEUE_H
 
 #include <condition_variable>  // condition_variable
 #include <cstdio>              // rename, remove
@@ -53,7 +53,7 @@ typedef std::function<void(ProcessingResult)> TFileProcessingFinishedCallback;
 constexpr char kCurrentFileName[] = "alohalytics_messages";
 constexpr char kArchivedFilesExtension[] = ".archived";
 
-class MessagesQueue {
+class MessagesQueue final {
  public:
   // Size limit (before gzip) when we archive "current" file and create a new
   // one for appending.
@@ -67,7 +67,7 @@ class MessagesQueue {
     std::rename(original_file.c_str(), out_archive.c_str());
   }
 
-  //
+  // Pass custom processing function here, e.g. append IDs, gzip everything before archiving file etc.
   MessagesQueue(TFileArchiver file_archiver = &ArchiveFileByRenamingIt) : file_archiver_(file_archiver) {}
 
   ~MessagesQueue() {
@@ -136,7 +136,7 @@ class MessagesQueue {
   void StoreMessages(std::string const & messages_buffer) {
     if (current_file_) {
       *current_file_ << messages_buffer << std::flush;
-      if (current_file_->tellp() > kMaxFileSizeInBytes) {
+      if (current_file_->tellp() >= kMaxFileSizeInBytes) {
         ArchiveCurrentFile();
       }
     } else {
@@ -209,7 +209,7 @@ class MessagesQueue {
       if (full_path_to_file.find(kArchivedFilesExtension) == std::string::npos) {
         return true;
       }
-      if (processor(true /* file path */, full_path_to_file)) {
+      if (processor(true /* true here means that second parameter is file path */, full_path_to_file)) {
         result = ProcessingResult::EProcessedSuccessfully;
         // Also delete successfully processed archive.
         std::remove(full_path_to_file.c_str());
@@ -256,7 +256,7 @@ class MessagesQueue {
   typedef std::function<void()> TCommand;
   std::list<TCommand> commands_queue_;
 
-  bool worker_thread_should_exit_ = false;
+  volatile bool worker_thread_should_exit_ = false;
   std::mutex mutex_;
   std::condition_variable condition_variable_;
   // Only WorkerThread accesses this variable.
@@ -268,4 +268,4 @@ class MessagesQueue {
 
 }  // namespace alohalytics
 
-#endif  // MESSAGES_QUEUE_HPP
+#endif  // MESSAGES_QUEUE_H
diff --git a/3party/Alohalytics/src/windows/file_manager_windows_impl.cc b/3party/Alohalytics/src/windows/file_manager_windows_impl.cc
index 908914f0ce..47985252dc 100644
--- a/3party/Alohalytics/src/windows/file_manager_windows_impl.cc
+++ b/3party/Alohalytics/src/windows/file_manager_windows_impl.cc
@@ -42,7 +42,7 @@ struct ScopedCloseFindFileHandle {
 }  // namespace
 
 void FileManager::ForEachFileInDir(std::string directory, std::function<bool(const std::string & full_path)> lambda) {
-  // Silently ignore invalid directories.
+  // Silently ignore invalid (empty) directories.
   if (directory.empty()) {
     return;
   }
diff --git a/3party/Alohalytics/tests/test_messages_queue.cc b/3party/Alohalytics/tests/test_messages_queue.cc
index 9735486a63..44b26fdc98 100644
--- a/3party/Alohalytics/tests/test_messages_queue.cc
+++ b/3party/Alohalytics/tests/test_messages_queue.cc
@@ -205,7 +205,16 @@ using alohalytics::ProcessingResult;
 
 bool EndsWith(const std::string & str, const std::string & suffix) {
   const std::string::size_type str_size = str.size(), suffix_size = suffix.size();
-  return str_size >= suffix_size && str.find_last_of(suffix) == str_size - suffix_size;
+  return str_size >= suffix_size && std::equal(suffix.begin(), suffix.end(), str.end() - suffix_size);
+}
+
+void Test_EndsWith() {
+  TEST_EQUAL(true, EndsWith("", ""));
+  TEST_EQUAL(true, EndsWith("Hello, World!", " World!"));
+  TEST_EQUAL(true, EndsWith("Hello", "Hello"));
+  TEST_EQUAL(false, EndsWith("Hello, World!", " World! "));
+  TEST_EQUAL(false, EndsWith("Hell", "Hello"));
+  TEST_EQUAL(false, EndsWith("ello", "Hello"));
 }
 
 // Removes all MessagesQueue's files in the directory.
@@ -390,9 +399,9 @@ void Test_MessagesQueue_CreateArchiveOnSizeLimitHit() {
     }
     size += generated_size;
   };
-  static const std::ofstream::pos_type limit = q.kMaxFileSizeInBytes / 2 + 100;
-  std::thread worker([&generator]() { generator(kTestWorkerMessage, limit); });
-  generator(kTestMessage, limit);
+  static const std::ofstream::pos_type number_of_bytes_to_generate = q.kMaxFileSizeInBytes / 2 + 100;
+  std::thread worker([&generator]() { generator(kTestWorkerMessage, number_of_bytes_to_generate); });
+  generator(kTestMessage, number_of_bytes_to_generate);
   worker.join();
 
   std::vector<std::ofstream::pos_type> file_sizes;
@@ -405,11 +414,7 @@ void Test_MessagesQueue_CreateArchiveOnSizeLimitHit() {
   TEST_EQUAL(ProcessingResult::EProcessedSuccessfully, finish_task.get());
   TEST_EQUAL(size_t(2), file_sizes.size());
   TEST_EQUAL(size, file_sizes[0] + file_sizes[1]);
-  if (file_sizes[0] < q.kMaxFileSizeInBytes) {
-    TEST_EQUAL(true, file_sizes[1] > q.kMaxFileSizeInBytes);
-  } else {
-    TEST_EQUAL(true, file_sizes[0] > q.kMaxFileSizeInBytes);
-  }
+  TEST_EQUAL(true, (file_sizes[0] > q.kMaxFileSizeInBytes) != (file_sizes[1] > q.kMaxFileSizeInBytes));
 }
 
 void Test_MessagesQueue_HighLoadAndIntegrity() {
@@ -422,7 +427,7 @@ void Test_MessagesQueue_HighLoadAndIntegrity() {
   MessagesQueue q;
   const int kMaxThreads = 300;
   std::mt19937 gen(std::mt19937::default_seed);
-  std::uniform_int_distribution<> dis(1, std::numeric_limits<char>::max());
+  std::uniform_int_distribution<> dis('A', 'Z');
   auto const generator = [&q](char c) { q.PushMessage(std::string(static_cast<size_t>(c), c)); };
   std::vector<std::thread> threads;
   size_t total_size = 0;
@@ -462,6 +467,7 @@ void Test_MessagesQueue_HighLoadAndIntegrity() {
 }
 
 int main(int, char * []) {
+  // TODO(AlexZ): Split unit tests into two separate files.
   Test_ScopedRemoveFile();
   Test_GetDirectoryFromFilePath();
   Test_CreateTemporaryFile();
@@ -470,6 +476,7 @@ int main(int, char * []) {
   Test_ForEachFileInDir();
   Test_GetFileSize();
 
+  Test_EndsWith();
   Test_MessagesQueue_InMemory_Empty();
   Test_MessagesQueue_InMemory_SuccessfulProcessing();
   Test_MessagesQueue_InMemory_FailedProcessing();