From d2ec26f494c96d9d36d082e4c8a0035fccbe99d1 Mon Sep 17 00:00:00 2001 From: Per Malmberg Date: Sun, 11 Mar 2018 15:26:26 +0100 Subject: [PATCH] Moce functionality in main Cron class with added tests. --- libcron/CMakeLists.txt | 6 +- libcron/Cron.cpp | 72 +++++++++++++++++++++--- libcron/Cron.h | 15 +++-- libcron/CronData.h | 1 + libcron/CronSchedule.h | 2 + libcron/Task.cpp | 51 ++++++++++++++++- libcron/Task.h | 28 +++++++--- test/CMakeLists.txt | 2 +- test/CronTest.cpp | 122 +++++++++++++++++++++++++++++++++++++---- 9 files changed, 263 insertions(+), 36 deletions(-) diff --git a/libcron/CMakeLists.txt b/libcron/CMakeLists.txt index 2e1882d..7287fc1 100644 --- a/libcron/CMakeLists.txt +++ b/libcron/CMakeLists.txt @@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.6) project(libcron) set(CMAKE_CXX_STANDARD 14) -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wpedantic -fsanitize=address") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wpedantic") include_directories(externals/date) @@ -15,4 +15,6 @@ add_library(${PROJECT_NAME} CronData.cpp CronSchedule.cpp CronSchedule.h - externals/date/date.h DateTime.h Task.cpp) + externals/date/date.h + DateTime.h + Task.cpp) diff --git a/libcron/Cron.cpp b/libcron/Cron.cpp index 7ea7ca6..66ae942 100644 --- a/libcron/Cron.cpp +++ b/libcron/Cron.cpp @@ -1,29 +1,87 @@ #include #include "Cron.h" +using namespace std::chrono; + namespace libcron { - - bool libcron::Cron::add_schedule(const std::string& schedule, std::function work) + bool libcron::Cron::add_schedule( std::string name, const std::string& schedule, std::function work) { auto cron = CronData::create(schedule); bool res = cron.is_valid(); if (res) { - CronSchedule s(cron); - Task t(std::move(s), std::move(work)); + + Task t{std::move(name), CronSchedule{cron}, std::move(work)}; if (t.calculate_next()) { - items.emplace(t); + tasks.push(t); } } return res; } - bool libcron::Cron::has_expired_task(const std::chrono::system_clock::time_point now) const + std::chrono::system_clock::duration Cron::time_until_next(std::chrono::system_clock::time_point now) const { - return !items.empty() && items.top().is_expired(now); + system_clock::duration d{}; + if (tasks.empty()) + { + d = std::numeric_limits::max(); + } + else + { + d = tasks.top().time_until_expiry(now); + } + + return d; + } + + size_t Cron::execute_expired_tasks(system_clock::time_point now) + { + std::vector executed{}; + + while(!tasks.empty() + && tasks.top().is_expired(now)) + { + executed.push_back(tasks.top()); + tasks.pop(); + auto& t = executed[executed.size()-1]; + t.execute(); + } + + auto res = executed.size(); + + // Place executed tasks back onto the priority queue. + std::for_each(executed.begin(), executed.end(), [this, &now](Task& task) + { + // Must calculate new schedules using second after 'now', otherwise + // we'll run the same task over and over if it takes less than 1s to execute. + if(task.calculate_next(now + 1s)) + { + tasks.push(task); + } + }); + + print_queue(tasks); + + return res; + } + + void Cron::print_queue(std::priority_queue, std::greater<>> queue) + { + std::vector v{}; + + while( !queue.empty()) + { + auto t = queue.top(); + queue.pop(); + v.push_back(t); + } + + std::for_each(v.begin(), v.end(), [&queue](auto& task){ + queue.push(task); + }); } } \ No newline at end of file diff --git a/libcron/Cron.h b/libcron/Cron.h index 52699c6..2cd5431 100644 --- a/libcron/Cron.h +++ b/libcron/Cron.h @@ -3,6 +3,7 @@ #include #include #include +#include #include "Task.h" namespace libcron @@ -11,16 +12,22 @@ namespace libcron { public: - bool add_schedule(const std::string& schedule, std::function work); + bool add_schedule(std::string name, const std::string& schedule, std::function work); size_t count() const { - return items.size(); + return tasks.size(); } - bool has_expired_task(std::chrono::system_clock::time_point now = std::chrono::system_clock::now()) const; + size_t + execute_expired_tasks(std::chrono::system_clock::time_point now = std::chrono::system_clock::now()); + + std::chrono::system_clock::duration + time_until_next(std::chrono::system_clock::time_point now = std::chrono::system_clock::now()) const; private: - std::priority_queue items{}; + // Priority queue placing smallest (i.e. nearest in time) items on top. + std::priority_queue, std::greater<>> tasks{}; + void print_queue(std::priority_queue, std::greater<>> queue); }; } \ No newline at end of file diff --git a/libcron/CronData.h b/libcron/CronData.h index 110bb5e..e9121a1 100644 --- a/libcron/CronData.h +++ b/libcron/CronData.h @@ -59,6 +59,7 @@ namespace libcron static CronData create(const std::string& cron_expression); CronData(); + CronData(const CronData&) = default; bool is_valid() const { diff --git a/libcron/CronSchedule.h b/libcron/CronSchedule.h index 629e3ac..15e9d88 100644 --- a/libcron/CronSchedule.h +++ b/libcron/CronSchedule.h @@ -15,6 +15,8 @@ namespace libcron { } + CronSchedule(const CronSchedule&) = default; + std::tuple calculate_from(const std::chrono::system_clock::time_point& from) const; diff --git a/libcron/Task.cpp b/libcron/Task.cpp index 51d124e..6aba733 100644 --- a/libcron/Task.cpp +++ b/libcron/Task.cpp @@ -1,17 +1,62 @@ +#include #include "Task.h" +using namespace std::chrono; + namespace libcron { bool Task::calculate_next(std::chrono::system_clock::time_point from) { auto result = schedule.calculate_from(from); - auto res = std::get<0>(result); - if(res) + + // In case the calculation fails, the task will no longer expire. + valid = std::get<0>(result); + if (valid) { next_schedule = std::get<1>(result); } - return res; + return valid; + } + + bool Task::is_expired(std::chrono::system_clock::time_point now) const + { + return valid && time_until_expiry(now) == 0s; + } + + std::chrono::system_clock::duration Task::time_until_expiry(std::chrono::system_clock::time_point now) const + { + system_clock::duration d{}; + + // Explicitly return 0s instead of a possibly negative duration when it has expired. + if (now >= next_schedule) + { + d = 0s; + } + else + { + d = next_schedule - now; + } + + return d; + } + + std::string Task::get_status(std::chrono::system_clock::time_point now) const + { + std::string s = "'"; + s+= get_name(); + s += "' expires in "; + s += std::to_string(duration_cast(time_until_expiry(now)).count()); + s += "ms => "; + + auto dt = CronSchedule::to_calendar_time(next_schedule); + s+= std::to_string(dt.year) + "-"; + s+= std::to_string(dt.month) + "-"; + s+= std::to_string(dt.day) + " "; + s+= std::to_string(dt.hour) + ":"; + s+= std::to_string(dt.min) + ":"; + s+= std::to_string(dt.sec) + " UTC"; + return s; } } \ No newline at end of file diff --git a/libcron/Task.h b/libcron/Task.h index 00bf981..d8060f9 100644 --- a/libcron/Task.h +++ b/libcron/Task.h @@ -11,30 +11,44 @@ namespace libcron { public: - Task(CronSchedule schedule, std::function task) - : schedule(std::move(schedule)), task(std::move(task)) + Task(const std::string name, const CronSchedule schedule, std::function task) + : name(std::move(name)), schedule(std::move(schedule)), task(std::move(task)) { } - Task(const Task&) = default; + void execute() const + { + task(); + } + + Task(const Task& other) = default; Task& operator=(const Task&) = default; bool calculate_next(std::chrono::system_clock::time_point from = std::chrono::system_clock::now()); - bool operator<(const Task& other) const + bool operator>(const Task& other) const { - return next_schedule < other.next_schedule; + return next_schedule > other.next_schedule; } - bool is_expired(std::chrono::system_clock::time_point now = std::chrono::system_clock::now()) const + bool is_expired(std::chrono::system_clock::time_point now = std::chrono::system_clock::now()) const; + + std::chrono::system_clock::duration + time_until_expiry(std::chrono::system_clock::time_point now = std::chrono::system_clock::now()) const; + + std::string get_name() const { - return now >= next_schedule; + return name; } + std::string get_status(std::chrono::system_clock::time_point now) const; + private: + std::string name; CronSchedule schedule; std::chrono::system_clock::time_point next_schedule; std::function task; + bool valid = false; }; } \ No newline at end of file diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 9470888..393c946 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.6) project(cron_test) set(CMAKE_CXX_STANDARD 14) -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wpedantic -fsanitize=address -lasan") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wpedantic -Wextra") include_directories( externals/Catch2/single_include/ diff --git a/test/CronTest.cpp b/test/CronTest.cpp index cb04ad9..1f79fad 100644 --- a/test/CronTest.cpp +++ b/test/CronTest.cpp @@ -1,6 +1,7 @@ #include #include #include +#include using namespace libcron; using namespace std::chrono; @@ -10,7 +11,7 @@ std::string create_schedule_expiring_in(hours h, minutes m, seconds s) auto now = system_clock::now() + h + m + s; auto dt = CronSchedule::to_calendar_time(now); - std::string res = ""; + std::string res{}; res += std::to_string(dt.sec) + " "; res += std::to_string(dt.min) + " "; res += std::to_string(dt.hour) + " * * *"; @@ -24,6 +25,7 @@ SCENARIO("Adding a task") GIVEN("A Cron instance with no task") { Cron c; + auto expired = false; THEN("Starts with no task") { @@ -32,23 +34,25 @@ SCENARIO("Adding a task") WHEN("Adding a task that runs every second") { - REQUIRE(c.add_schedule("* * * * * *", - []() + REQUIRE(c.add_schedule("A task", "* * * * * *", + [&expired]() { - return; + expired = true; }) ); THEN("Count is 1 and task was not expired two seconds ago") { REQUIRE(c.count() == 1); - REQUIRE_FALSE(c.has_expired_task(system_clock::now() - 2s)); + c.execute_expired_tasks(system_clock::now() - 2s); + REQUIRE_FALSE(expired); } AND_THEN("Task is expired when calculating based on current time") { + c.execute_expired_tasks(); THEN("Task is expired") { - REQUIRE(c.has_expired_task()); + REQUIRE(expired); } } } @@ -59,34 +63,128 @@ SCENARIO("Adding a task that expires in the future") { GIVEN("A Cron instance with task expiring in 3 seconds") { + auto expired = false; + Cron c; - REQUIRE(c.add_schedule(create_schedule_expiring_in(hours{0}, minutes{0}, seconds{3}), - []() + REQUIRE(c.add_schedule("A task", create_schedule_expiring_in(hours{0}, minutes{0}, seconds{3}), + [&expired]() { - return; + expired = true; }) ); THEN("Not yet expired") { - REQUIRE_FALSE(c.has_expired_task()); + REQUIRE_FALSE(expired); } AND_WHEN("When waiting one second") { std::this_thread::sleep_for(1s); + c.execute_expired_tasks(); THEN("Task has not yet expired") { - REQUIRE_FALSE(c.has_expired_task()); + REQUIRE_FALSE(expired); } } AND_WHEN("When waiting three seconds") { std::this_thread::sleep_for(3s); + c.execute_expired_tasks(); THEN("Task has expired") { - REQUIRE(c.has_expired_task()); + REQUIRE(expired); } } } } +SCENARIO("Task priority") +{ + GIVEN("A Cron instance with two tasks expiring in 3 and 5 seconds, added in 'reverse' order") + { + auto _3_second_expired = 0; + auto _5_second_expired = 0; + + + Cron c; + REQUIRE(c.add_schedule("Five", create_schedule_expiring_in(hours{0}, minutes{0}, seconds{5}), + [&_5_second_expired]() + { + _5_second_expired++; + }) + ); + + REQUIRE(c.add_schedule("Three", create_schedule_expiring_in(hours{0}, minutes{0}, seconds{3}), + [&_3_second_expired]() + { + _3_second_expired++; + }) + ); + + THEN("Not yet expired") + { + REQUIRE_FALSE(_3_second_expired); + REQUIRE_FALSE(_5_second_expired); + } + + WHEN("Waiting 1 seconds") + { + std::this_thread::sleep_for(1s); + c.execute_expired_tasks(); + + THEN("Task has not yet expired") + { + REQUIRE(_3_second_expired == 0); + REQUIRE(_5_second_expired == 0); + } + } + AND_WHEN("Waiting 3 seconds") + { + std::this_thread::sleep_for(3s); + c.execute_expired_tasks(); + + THEN("3 second task has expired") + { + REQUIRE(_3_second_expired == 1); + REQUIRE(_5_second_expired == 0); + } + } + AND_WHEN("Waiting 5 seconds") + { + std::this_thread::sleep_for(5s); + c.execute_expired_tasks(); + + THEN("3 and 5 second task has expired") + { + REQUIRE(_3_second_expired == 1); + REQUIRE(_5_second_expired == 1); + } + } + AND_WHEN("Waiting based on the time given by the Cron instance") + { + std::this_thread::sleep_for(c.time_until_next()); + c.execute_expired_tasks(); + + THEN("3 second task has expired") + { + REQUIRE(_3_second_expired == 1); + REQUIRE(_5_second_expired == 0); + } + } + AND_WHEN("Waiting based on the time given by the Cron instance") + { + std::this_thread::sleep_for(c.time_until_next()); + REQUIRE(c.execute_expired_tasks() == 1); + + std::this_thread::sleep_for(c.time_until_next()); + REQUIRE(c.execute_expired_tasks() == 1); + + THEN("3 and 5 second task has each expired once") + { + REQUIRE(_3_second_expired == 1); + REQUIRE(_5_second_expired == 1); + } + } + + } +} \ No newline at end of file