From 4ac4143302a05816529f3820d9a9d623947a6968 Mon Sep 17 00:00:00 2001 From: Marius Bancila Date: Wed, 12 Aug 2026 15:45:27 +0300 Subject: [PATCH] let traits decide how the two day fields combine --- README.md | 15 ++++ include/croncpp.h | 79 +++++++++++++++-- test/CMakeLists.txt | 2 +- test/test_day_fields.cpp | 177 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 267 insertions(+), 6 deletions(-) create mode 100644 test/test_day_fields.cpp diff --git a/README.md b/README.md index b055dfb..f794d43 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,21 @@ The special characters have the following meaning: `W` moves to the nearest Monday to Friday: back one day from a Saturday, forward one day from a Sunday. It never crosses into another month, so `1W` on a Saturday is the Monday after, and `31W` on a Sunday is the Friday before. The weekday numbers in `5L` and `5#2` follow the traits in use, so the last Friday is `5L` with `cron_standard_traits` and `6L` with `cron_quartz_traits`. +### The two day fields + +The days of month and days of week fields both select days, so an expression restricting both has to say what that means. The two dialects croncpp follows disagree, and each traits type states its own answer through `CRON_DAY_FIELD_RULE`: + +| Traits | Rule | Meaning | +| --- | --- | --- | +| `cron_standard_traits` | `day_field_rule::either` | a date matching **either** field is a match, as in POSIX cron | +| `cron_quartz_traits`, `cron_oracle_traits` | `day_field_rule::reject` | the expression is an error; one of the two fields has to be `?` | + +So `0 0 0 1 1 1` — midnight on the 1st of January, and on Mondays in January — resolves to the 1st of January 2021 under the standard traits, because that date matches the days of month half. Under the quartz and oracle traits the same expression is rejected, and `0 0 0 1 1 ?` or `0 0 0 ? 1 1` says which of the two was meant. + +A field counts as restricted unless it is exactly `*` or `?`. `1-31` covers every day but is still a restriction, and POSIX treats it as one. + +A traits type that does not declare `CRON_DAY_FIELD_RULE` gets `day_field_rule::intersect`, where a date has to match both fields. That was croncpp's behaviour before the rule existed, so custom traits written against an earlier version are unaffected. + The `years` field is optional, as in Quartz, and may be left out entirely. An expression whose years have all gone by has no next occurrence, so `cron_next()` reports failure for it: `INVALID_TIME` from the `std::time_t` overload, and a zeroed `std::tm` from the other. A traits type opts into the year field by declaring `CRON_MIN_YEARS` and `CRON_MAX_YEARS`, as all three supplied ones do. A traits type written without them keeps accepting six fields and rejects a seventh, so custom traits written against an earlier version of croncpp continue to work unchanged. Note that the range croncpp can store is fixed at 1970-2099 whatever the traits say, so a traits type may narrow that range but not widen it. diff --git a/include/croncpp.h b/include/croncpp.h index 11ebb1c..837bf98 100644 --- a/include/croncpp.h +++ b/include/croncpp.h @@ -54,6 +54,25 @@ namespace cron struct supports_years> : std::true_type {}; + // What an expression means when it restricts both the day of month and the + // day of week, which the two dialects croncpp follows answer differently. + enum class day_field_rule + { + intersect, // a date has to match both fields + either, // a date matching either field is a match, as in POSIX cron + reject // the expression is an error, as in Quartz + }; + + // A traits type states its rule by declaring CRON_DAY_FIELD_RULE. One that + // does not, as any written before the rule existed, keeps intersecting. + template + struct day_rule + : std::integral_constant {}; + + template + struct day_rule> + : std::integral_constant {}; + class cronexpr; namespace detail @@ -83,12 +102,14 @@ namespace cron bool last = false; // L, the last day of the month bool nearest_weekday = false; // W, the nearest Monday to Friday cron_int day = 0; // the day W applies to, 0 for LW + bool restricted = false; // the field was neither * nor ? }; struct day_of_week_options { - cron_int nth = 0; // #, the 1st to 5th such weekday of the month - bool last = false; // L, the last such weekday of the month + cron_int nth = 0; // #, the 1st to 5th such weekday of the month + bool last = false; // L, the last such weekday of the month + bool restricted = false; // the field was neither * nor ? }; } @@ -123,6 +144,9 @@ namespace cron static const cron_int CRON_MAX_YEARS_DIFF = 4; + // POSIX cron: with both day fields restricted, either may match + static const day_field_rule CRON_DAY_FIELD_RULE = day_field_rule::either; + static const int CRON_MIN_YEARS = 1970; static const int CRON_MAX_YEARS = 2099; @@ -166,6 +190,9 @@ namespace cron static const cron_int CRON_MAX_YEARS_DIFF = 4; + // as in Quartz, one of the two day fields has to be ? + static const day_field_rule CRON_DAY_FIELD_RULE = day_field_rule::reject; + static const int CRON_MIN_YEARS = 1970; static const int CRON_MAX_YEARS = 2099; @@ -210,6 +237,9 @@ namespace cron static const cron_int CRON_MAX_YEARS_DIFF = 4; + // Quartz requires one of the two day fields to be ? + static const day_field_rule CRON_DAY_FIELD_RULE = day_field_rule::reject; + static const int CRON_MIN_YEARS = 1970; static const int CRON_MAX_YEARS = 2099; @@ -298,8 +328,10 @@ namespace cron e1.dom_options.last == e2.dom_options.last && e1.dom_options.nearest_weekday == e2.dom_options.nearest_weekday && e1.dom_options.day == e2.dom_options.day && + e1.dom_options.restricted == e2.dom_options.restricted && e1.dow_options.nth == e2.dow_options.nth && - e1.dow_options.last == e2.dow_options.last; + e1.dow_options.last == e2.dow_options.last && + e1.dow_options.restricted == e2.dow_options.restricted; } inline bool operator!=(cronexpr const & e1, cronexpr const & e2) @@ -638,6 +670,11 @@ namespace cron if (days.size() == 1 && days[0] == '?') days[0] = '*'; + // Whether the field names any particular day at all. Only * and ? + // leave it open; a list, a range, or even 1-31 restricts it, which is + // the distinction POSIX cron draws between the two day fields. + options.restricted = !(days.size() == 1 && days[0] == '*'); + // On its own, L means Saturday, as in Quartz. After a weekday it // means the last such weekday of the month, and # selects which one. if (days == "L") @@ -702,6 +739,8 @@ namespace cron if (days.size() == 1 && days[0] == '?') days[0] = '*'; + options.restricted = !(days.size() == 1 && days[0] == '*'); + if (utils::contains(days, 'L') || utils::contains(days, 'W')) { if (has_list_or_range(days)) @@ -1144,6 +1183,29 @@ namespace cron return true; } + // Combines the two day fields according to the rule of the dialect the + // traits describe. An unrestricted field matches every day, so only the + // case where both name particular days can differ between the rules. + template + inline bool matches_day( + std::tm const & date, + std::bitset<31> const & days_of_month, + day_of_month_options const & dom_options, + std::bitset<7> const & days_of_week, + day_of_week_options const & dow_options) + { + bool const day_of_month = + matches_day_of_month(date, days_of_month, dom_options); + bool const day_of_week = + matches_day_of_week(date, days_of_week, dow_options); + + if (day_rule::value == day_field_rule::either && + dom_options.restricted && dow_options.restricted) + return day_of_month || day_of_week; + + return day_of_month && day_of_week; + } + template static size_t find_next_day( std::tm& date, @@ -1162,8 +1224,8 @@ namespace cron // CRON_MIN_DAYS_OF_WEEK and the bits are set at // value - CRON_MIN_DAYS_OF_WEEK. while ( - !(matches_day_of_month(date, days_of_month, dom_options) && - matches_day_of_week(date, days_of_week, dow_options)) + !matches_day( + date, days_of_month, dom_options, days_of_week, dow_options) && count++ < maximum) { add_to_field(date, cron_field::day_of_month, 1); @@ -1397,6 +1459,13 @@ namespace cron detail::set_cron_days_of_month(fields[3], cex.days_of_month, cex.dom_options); + // Quartz, and the Oracle format that follows it, require one of the two + // day fields to be left open with ? + if (day_rule::value == day_field_rule::reject && + cex.dom_options.restricted && cex.dow_options.restricted) + throw bad_cronexpr( + "Specify a day of month or a day of week, and ? for the other"); + detail::set_cron_month(fields[4], cex.months); if (fields.size() == 7) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index ee26233..03443fd 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -1,4 +1,4 @@ -set(SOURCES main.cpp test_dst.cpp test_oracle.cpp test_quartz.cpp test_special.cpp test_standard.cpp test_years.cpp) +set(SOURCES main.cpp test_day_fields.cpp test_dst.cpp test_oracle.cpp test_quartz.cpp test_special.cpp test_standard.cpp test_years.cpp) add_executable(test_croncpp ${SOURCES}) diff --git a/test/test_day_fields.cpp b/test/test_day_fields.cpp new file mode 100644 index 0000000..717cd08 --- /dev/null +++ b/test/test_day_fields.cpp @@ -0,0 +1,177 @@ +#include "catch.hpp" +#include "croncpp.h" + +#include +#include + +using namespace cron; + +// How the day of month and day of week fields combine when an expression +// restricts both of them. POSIX cron matches a date against either field, +// Quartz refuses the expression and wants ? in one of them, and a traits type +// that says nothing keeps intersecting the two. + +namespace +{ + struct traits_without_rule + { + static const cron_int CRON_MIN_SECONDS = 0; + static const cron_int CRON_MAX_SECONDS = 59; + + static const cron_int CRON_MIN_MINUTES = 0; + static const cron_int CRON_MAX_MINUTES = 59; + + static const cron_int CRON_MIN_HOURS = 0; + static const cron_int CRON_MAX_HOURS = 23; + + static const cron_int CRON_MIN_DAYS_OF_WEEK = 0; + static const cron_int CRON_MAX_DAYS_OF_WEEK = 6; + + static const cron_int CRON_MIN_DAYS_OF_MONTH = 1; + static const cron_int CRON_MAX_DAYS_OF_MONTH = 31; + + static const cron_int CRON_MIN_MONTHS = 1; + static const cron_int CRON_MAX_MONTHS = 12; + + static const cron_int CRON_MAX_YEARS_DIFF = 4; + +#ifdef CRONCPP_IS_CPP17 + static const inline std::vector DAYS = { "SUN", "MON", "TUE", "WED", "THU", "FRI", "SAT" }; + static const inline std::vector MONTHS = { "NIL", "JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC" }; +#else + static std::vector& DAYS() + { + static std::vector days = { "SUN", "MON", "TUE", "WED", "THU", "FRI", "SAT" }; + return days; + } + + static std::vector& MONTHS() + { + static std::vector months = { "NIL", "JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC" }; + return months; + } +#endif + }; + + struct traits_rejecting : traits_without_rule + { + static const day_field_rule CRON_DAY_FIELD_RULE = day_field_rule::reject; + }; + + struct traits_either : traits_without_rule + { + static const day_field_rule CRON_DAY_FIELD_RULE = day_field_rule::either; + }; + + template + std::string next_str(std::string const & expr, std::string const & from) + { + auto cex = make_cron(expr); + auto date = utils::to_tm(from); + + return utils::to_string(cron_next(cex, date)); + } + + std::string next(std::string const & expr, std::string const & from) + { + return next_str(expr, from); + } +} + +TEST_CASE("day fields: issue 12, either field may match under POSIX rules", "[days]") +{ + // the 1st of January 2021 is a Friday, and it is the 1st, so it matches the + // day of month half of "0 0 0 1 1 1" + REQUIRE(next("0 0 0 1 1 1", "2020-10-08 00:00:00") == "2021-01-01 00:00:00"); + + // and the Mondays of January match the other half + REQUIRE(next("0 0 0 1 1 1", "2021-01-01 00:00:00") == "2021-01-04 00:00:00"); + REQUIRE(next("0 0 0 1 1 1", "2021-01-04 00:00:00") == "2021-01-11 00:00:00"); + + // the 15th of any month, or any Monday + REQUIRE(next("0 0 0 15 * 1", "2021-02-28 00:00:00") == "2021-03-01 00:00:00"); // a Monday + REQUIRE(next("0 0 0 15 * 1", "2021-03-01 00:00:00") == "2021-03-08 00:00:00"); // a Monday + REQUIRE(next("0 0 0 15 * 1", "2021-03-09 00:00:00") == "2021-03-15 00:00:00"); // both + REQUIRE(next("0 0 0 15 * 1", "2021-04-13 00:00:00") == "2021-04-15 00:00:00"); // the 15th, a Thursday +} + +TEST_CASE("day fields: a field is restricted unless it is * or ?", "[days]") +{ + // 1-31 covers every day but is still a restriction, so the rule applies and + // the expression matches every day of January rather than only Mondays + REQUIRE(next("0 0 0 1-31 1 1", "2020-12-31 00:00:00") == "2021-01-01 00:00:00"); + REQUIRE(next("0 0 0 1-31 1 1", "2021-01-01 00:00:00") == "2021-01-02 00:00:00"); + + // with the day of month left open, only Mondays match + REQUIRE(next("0 0 0 * 1 1", "2020-12-31 00:00:00") == "2021-01-04 00:00:00"); + + // so the two are not the same expression + REQUIRE(make_cron("0 0 0 1-31 1 1") != make_cron("0 0 0 * 1 1")); +} + +TEST_CASE("day fields: one field open behaves as before", "[days]") +{ + REQUIRE(next("0 0 0 1 1 ?", "2020-10-08 00:00:00") == "2021-01-01 00:00:00"); + REQUIRE(next("0 0 0 ? 1 1", "2020-10-08 00:00:00") == "2021-01-04 00:00:00"); + REQUIRE(next("0 0 0 1 1 *", "2020-10-08 00:00:00") == "2021-01-01 00:00:00"); + REQUIRE(next("0 0 0 * 1 *", "2020-12-31 00:00:00") == "2021-01-01 00:00:00"); + + // ? and * mean the same thing in either field + REQUIRE(make_cron("0 0 0 1 1 ?") == make_cron("0 0 0 1 1 *")); + REQUIRE(make_cron("0 0 0 ? 1 1") == make_cron("0 0 0 * 1 1")); +} + +TEST_CASE("day fields: quartz and oracle refuse to guess", "[days]") +{ + REQUIRE_THROWS_AS(make_cron("0 0 0 1 1 1"), bad_cronexpr); + REQUIRE_THROWS_AS(make_cron("0 0 0 1 1 1"), bad_cronexpr); + REQUIRE_THROWS_AS(make_cron("0 0 0 1-15 * 2-6"), bad_cronexpr); + + // a day of month qualified with L or W is a restriction too + REQUIRE_THROWS_AS(make_cron("0 0 0 L * 6"), bad_cronexpr); + REQUIRE_THROWS_AS(make_cron("0 0 0 15W * 6"), bad_cronexpr); + + // leaving one of them open is what quartz asks for + REQUIRE_NOTHROW(make_cron("0 0 0 1 1 ?")); + REQUIRE_NOTHROW(make_cron("0 0 0 ? 1 1")); + REQUIRE_NOTHROW(make_cron("0 0 0 * * *")); + REQUIRE_NOTHROW(make_cron("0 0 0 L * ?")); + REQUIRE_NOTHROW(make_cron("0 0 0 ? * 6#2")); + REQUIRE_NOTHROW(make_cron("0 0 0 ? 1 1")); +} + +TEST_CASE("day fields: the rule comes from the traits", "[days]") +{ + static_assert(day_rule::value == day_field_rule::either, + "the standard traits follow POSIX"); + static_assert(day_rule::value == day_field_rule::reject, + "quartz wants ? in one field"); + static_assert(day_rule::value == day_field_rule::reject, + "so does the oracle format"); + static_assert(day_rule::value == day_field_rule::intersect, + "traits saying nothing keep intersecting"); + + REQUIRE(day_rule::value == day_field_rule::either); + REQUIRE(day_rule::value == day_field_rule::reject); +} + +TEST_CASE("day fields: traits that state no rule are unaffected", "[days]") +{ + // both fields have to match, which is what croncpp did before the rule + // existed: the first of January that is also a Monday + REQUIRE(next_str("0 0 0 1 1 1", "2020-10-08 00:00:00") + == "2024-01-01 00:00:00"); + + // the same expression under each of the other two rules + REQUIRE(next_str("0 0 0 1 1 1", "2020-10-08 00:00:00") + == "2021-01-01 00:00:00"); + REQUIRE_THROWS_AS(make_cron("0 0 0 1 1 1"), bad_cronexpr); + + // and an expression leaving one field open means the same under all three + REQUIRE(next_str("0 0 0 1 1 ?", "2020-10-08 00:00:00") + == "2021-01-01 00:00:00"); + REQUIRE(next_str("0 0 0 1 1 ?", "2020-10-08 00:00:00") + == "2021-01-01 00:00:00"); + REQUIRE(next_str("0 0 0 1 1 ?", "2020-10-08 00:00:00") + == "2021-01-01 00:00:00"); +}