forked from organicmaps/organicmaps
[alohalytics] More review fixes.
This commit is contained in:
parent
83b7454b0c
commit
451d1b6420
9 changed files with 42 additions and 29 deletions
|
@ -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
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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));
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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();
|
||||
|
|
Loading…
Add table
Reference in a new issue