martinzink commented on code in PR #1352:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1352#discussion_r903635897
##########
libminifi/include/utils/TimeUtil.h:
##########
@@ -86,125 +86,49 @@ class SteadyClock : public Clock {
}
};
-/**
- * Returns a string based on TIME_FORMAT, converting
- * the parameter to a string
- * @param msec milliseconds since epoch
- * @returns string representing the time
- */
-inline std::string getTimeStr(uint64_t msec, bool enforce_locale = false) {
- char date[120];
- time_t second = (time_t) (msec / 1000);
- msec = msec % 1000;
- strftime(date, sizeof(date) / sizeof(*date), TIME_FORMAT, (enforce_locale ==
true ? gmtime(&second) : localtime(&second)));
-
- std::string ret = date;
- date[0] = '\0';
- sprintf(date, ".%03llu", (unsigned long long) msec); // NOLINT
-
- ret += date;
- return ret;
+inline std::string getTimeStr(std::chrono::system_clock::time_point tp) {
+ std::ostringstream stream;
+ date::to_stream(stream, TIME_FORMAT,
std::chrono::floor<std::chrono::milliseconds>(tp));
+ return stream.str();
}
-inline time_t mkgmtime(struct tm* date_time) {
-#ifdef WIN32
- return _mkgmtime(date_time);
-#else
- static const int month_lengths[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31,
30, 31};
- static const int month_lengths_leap[] = {31, 29, 31, 30, 31, 30, 31, 31, 30,
31, 30, 31};
- static const auto is_leap_year = [](int year) -> bool {
- return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
- };
- static const auto num_leap_days = [](int year) -> int {
- return (year - 1968) / 4 - (year - 1900) / 100 + (year - 1600) / 400;
- };
-
- int year = date_time->tm_year + 1900;
- time_t result = year - 1970;
- result *= 365;
- result += num_leap_days(year - 1);
-
- for (int i = 0; i < 12 && i < date_time->tm_mon; ++i) {
- result += is_leap_year(year) ? month_lengths_leap[i] : month_lengths[i];
- }
-
- result += date_time->tm_mday - 1;
- result *= 24;
-
- result += date_time->tm_hour;
- result *= 60;
-
- result += date_time->tm_min;
- result *= 60;
-
- result += date_time->tm_sec;
- return result;
-#endif
+template<class TimePoint>
+inline std::optional<TimePoint> parseDateTimeStr(const std::string& str) {
Review Comment:
makes sense, removed the template in
https://github.com/apache/nifi-minifi-cpp/pull/1352/commits/68929f499f8dc41248d06c564ca9ed21f84609a1#diff-4a76905d55704437ae0a7a4ee434a73f057c105c97bdf8756f5747b4fbc65e9fL105
##########
libminifi/test/unit/TimeUtilTests.cpp:
##########
@@ -21,79 +21,60 @@
using namespace std::literals::chrono_literals;
-namespace {
- constexpr int ONE_HOUR = 60 * 60;
- constexpr int ONE_DAY = 24 * ONE_HOUR;
-
- struct tm createTm(int year, int month, int day, int hour, int minute, int
second, bool is_dst = false) {
- struct tm date_time;
- date_time.tm_year = year - 1900;
- date_time.tm_mon = month - 1;
- date_time.tm_mday = day;
- date_time.tm_hour = hour;
- date_time.tm_min = minute;
- date_time.tm_sec = second;
- date_time.tm_isdst = is_dst ? 1 : 0;
- return date_time;
- }
-
- void mkgmtimeTestHelper(time_t expected, int year, int month, int day, int
hour, int minute, int second) {
- using org::apache::nifi::minifi::utils::timeutils::mkgmtime;
- struct tm date_time = createTm(year, month, day, hour, minute, second);
- REQUIRE(mkgmtime(&date_time) == expected);
- }
-} // namespace
+template <class TimePointType>
+void testParseDateTimeStr() {
+ using org::apache::nifi::minifi::utils::timeutils::parseDateTimeStr;
-TEST_CASE("mkgmtime() works correctly", "[mkgmtime]") {
- mkgmtimeTestHelper(0, 1970, 1, 1, 0, 0, 0);
- for (int hour = 0; hour < 24; ++hour) {
- mkgmtimeTestHelper((hour + 1) * ONE_HOUR - 1, 1970, 1, 1, hour, 59, 59);
- }
-
- mkgmtimeTestHelper(ONE_DAY, 1970, 1, 2, 0, 0, 0);
- mkgmtimeTestHelper(31 * ONE_DAY, 1970, 2, 1, 0, 0, 0);
- mkgmtimeTestHelper(365 * ONE_DAY, 1971, 1, 1, 0, 0, 0);
-
- mkgmtimeTestHelper(793929600, 1995, 2, 28, 0, 0, 0);
- mkgmtimeTestHelper(793929600 + ONE_DAY, 1995, 3, 1, 0, 0, 0);
- mkgmtimeTestHelper(825465600, 1996, 2, 28, 0, 0, 0);
- mkgmtimeTestHelper(825465600 + ONE_DAY, 1996, 2, 29, 0, 0, 0);
- mkgmtimeTestHelper(951696000, 2000, 2, 28, 0, 0, 0);
- mkgmtimeTestHelper(951696000 + ONE_DAY, 2000, 2, 29, 0, 0, 0);
- mkgmtimeTestHelper(4107456000, 2100, 2, 28, 0, 0, 0);
- mkgmtimeTestHelper(4107456000 + ONE_DAY, 2100, 3, 1, 0, 0, 0);
-
- mkgmtimeTestHelper(1513104856, 2017, 12, 12, 18, 54, 16);
- mkgmtimeTestHelper(1706655675, 2024, 1, 30, 23, 01, 15);
- mkgmtimeTestHelper(3710453630, 2087, 7, 31, 01, 33, 50);
+ CHECK(*parseDateTimeStr<TimePointType>("1970-01-01T00:00:00Z") ==
TimePointType{0s});
+ CHECK(*parseDateTimeStr<TimePointType>("1970-01-01T00:59:59Z") ==
TimePointType{1h - 1s});
+
+ CHECK(*parseDateTimeStr<TimePointType>("1970-01-02T00:00:00Z") ==
TimePointType{std::chrono::days(1)});
+ CHECK(*parseDateTimeStr<TimePointType>("1970-02-01T00:00:00Z") ==
TimePointType{31 * std::chrono::days(1)});
+ CHECK(*parseDateTimeStr<TimePointType>("1971-01-01T00:00:00Z") ==
TimePointType{365 * std::chrono::days(1)});
+
+ CHECK(*parseDateTimeStr<TimePointType>("1995-02-28T00:00:00Z") ==
TimePointType{793929600s});
+ CHECK(*parseDateTimeStr<TimePointType>("1995-03-01T00:00:00Z") ==
TimePointType{793929600s + std::chrono::days(1)});
+ CHECK(*parseDateTimeStr<TimePointType>("1996-02-28T00:00:00Z") ==
TimePointType{825465600s});
+ CHECK(*parseDateTimeStr<TimePointType>("1996-02-29T00:00:00Z") ==
TimePointType{825465600s + std::chrono::days(1)});
+ CHECK(*parseDateTimeStr<TimePointType>("2000-02-28T00:00:00Z") ==
TimePointType{951696000s});
+ CHECK(*parseDateTimeStr<TimePointType>("2000-02-29T00:00:00Z") ==
TimePointType{951696000s + std::chrono::days(1)});
+ CHECK(*parseDateTimeStr<TimePointType>("2100-02-28T00:00:00Z") ==
TimePointType{4107456000s});
+ CHECK(*parseDateTimeStr<TimePointType>("2100-03-01T00:00:00Z") ==
TimePointType{4107456000s + std::chrono::days(1)});
+
+ CHECK(*parseDateTimeStr<TimePointType>("2017-12-12T18:54:16Z") ==
TimePointType{1513104856s});
+ CHECK(*parseDateTimeStr<TimePointType>("2024-01-30T23:01:15Z") ==
TimePointType{1706655675s});
+ CHECK(*parseDateTimeStr<TimePointType>("2087-07-31T01:33:50Z") ==
TimePointType{3710453630s});
}
TEST_CASE("parseDateTimeStr() works correctly", "[parseDateTimeStr]") {
Review Comment:
:+1:
https://github.com/apache/nifi-minifi-cpp/pull/1352/commits/68929f499f8dc41248d06c564ca9ed21f84609a1#diff-821242a5468ace9f4c44a7d4c13d422cdea2d26de1ac3f6d1be4090e750427d4R48-R69
##########
libminifi/include/utils/TimeUtil.h:
##########
@@ -86,125 +86,49 @@ class SteadyClock : public Clock {
}
};
-/**
- * Returns a string based on TIME_FORMAT, converting
- * the parameter to a string
- * @param msec milliseconds since epoch
- * @returns string representing the time
- */
-inline std::string getTimeStr(uint64_t msec, bool enforce_locale = false) {
- char date[120];
- time_t second = (time_t) (msec / 1000);
- msec = msec % 1000;
- strftime(date, sizeof(date) / sizeof(*date), TIME_FORMAT, (enforce_locale ==
true ? gmtime(&second) : localtime(&second)));
-
- std::string ret = date;
- date[0] = '\0';
- sprintf(date, ".%03llu", (unsigned long long) msec); // NOLINT
-
- ret += date;
- return ret;
+inline std::string getTimeStr(std::chrono::system_clock::time_point tp) {
+ std::ostringstream stream;
+ date::to_stream(stream, TIME_FORMAT,
std::chrono::floor<std::chrono::milliseconds>(tp));
+ return stream.str();
}
-inline time_t mkgmtime(struct tm* date_time) {
-#ifdef WIN32
- return _mkgmtime(date_time);
-#else
- static const int month_lengths[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31,
30, 31};
- static const int month_lengths_leap[] = {31, 29, 31, 30, 31, 30, 31, 31, 30,
31, 30, 31};
- static const auto is_leap_year = [](int year) -> bool {
- return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
- };
- static const auto num_leap_days = [](int year) -> int {
- return (year - 1968) / 4 - (year - 1900) / 100 + (year - 1600) / 400;
- };
-
- int year = date_time->tm_year + 1900;
- time_t result = year - 1970;
- result *= 365;
- result += num_leap_days(year - 1);
-
- for (int i = 0; i < 12 && i < date_time->tm_mon; ++i) {
- result += is_leap_year(year) ? month_lengths_leap[i] : month_lengths[i];
- }
-
- result += date_time->tm_mday - 1;
- result *= 24;
-
- result += date_time->tm_hour;
- result *= 60;
-
- result += date_time->tm_min;
- result *= 60;
-
- result += date_time->tm_sec;
- return result;
-#endif
+template<class TimePoint>
+inline std::optional<TimePoint> parseDateTimeStr(const std::string& str) {
+ std::istringstream stream(str);
+ TimePoint tp;
+ date::from_stream(stream, "%Y-%m-%dT%H:%M:%SZ", tp);
+ if (stream.fail() || (stream.peek() && !stream.eof()))
+ return std::nullopt;
+ return tp;
}
-/**
- * Parse a datetime in yyyy-MM-dd'T'HH:mm:ssZ format
- * @param str the datetime string
- * @returns Unix timestamp
- */
-inline int64_t parseDateTimeStr(const std::string& str) {
- /**
- * There is no strptime on Windows. As long as we have to parse a single
date format this is not so bad,
- * but if multiple formats will have to be supported in the future, it might
be worth it to include
- * an strptime implementation from some BSD on Windows.
- */
- uint32_t year;
- uint8_t month;
- uint8_t day;
- uint8_t hours;
- uint8_t minutes;
- uint8_t seconds;
- int read = 0;
- if (sscanf(str.c_str(), "%4u-%2hhu-%2hhuT%2hhu:%2hhu:%2hhuZ%n", &year,
&month, &day, &hours, &minutes, &seconds, &read) != 6) {
- return -1;
- }
- // while it is unlikely that read will be < 0, the conditional adds little
cost for a little defensiveness.
- if (read < 0 || static_cast<size_t>(read) != str.size()) {
- return -1;
- }
-
- if (year < 1970U ||
- month > 12U ||
- day > 31U ||
- hours > 23U ||
- minutes > 59U ||
- seconds > 60U) {
- return -1;
- }
-
- struct tm timeinfo{};
- timeinfo.tm_year = year - 1900;
- timeinfo.tm_mon = month - 1;
- timeinfo.tm_mday = day;
- timeinfo.tm_hour = hours;
- timeinfo.tm_min = minutes;
- timeinfo.tm_sec = seconds;
- timeinfo.tm_isdst = 0;
-
- return static_cast<int64_t>(mkgmtime(&timeinfo));
+template<class TimePoint>
+inline std::optional<std::string> getDateTimeStr(TimePoint tp) {
+ std::ostringstream stream;
+ date::to_stream(stream, "%Y-%m-%dT%H:%M:%SZ",
std::chrono::floor<std::chrono::milliseconds>(tp));
+ if (stream.fail())
+ return std::nullopt;
Review Comment:
you are right, this shouldnt fail, changed it in
https://github.com/apache/nifi-minifi-cpp/pull/1352/commits/68929f499f8dc41248d06c564ca9ed21f84609a1#diff-4a76905d55704437ae0a7a4ee434a73f057c105c97bdf8756f5747b4fbc65e9fR104
##########
extensions/sftp/processors/ListSFTP.cpp:
##########
@@ -63,6 +63,21 @@ constexpr char const*
ListSFTP::TARGET_SYSTEM_TIMESTAMP_PRECISION_MINUTES;
constexpr char const*
ListSFTP::ENTITY_TRACKING_INITIAL_LISTING_TARGET_TRACKING_TIME_WINDOW;
constexpr char const*
ListSFTP::ENTITY_TRACKING_INITIAL_LISTING_TARGET_ALL_AVAILABLE;
+namespace {
+uint64_t createTimestamp(const
std::optional<std::chrono::system_clock::time_point> time_point) {
+ if (!time_point)
+ return 0;
+ return
std::chrono::duration_cast<std::chrono::milliseconds>(time_point->time_since_epoch()).count();
+}
+
+std::optional<std::chrono::system_clock::time_point> parseTimestamp(const
uint64_t timestamp) {
+ if (timestamp == 0)
+ return std::nullopt;
+ return
std::chrono::system_clock::time_point{std::chrono::milliseconds{timestamp}};
+}
Review Comment:
good idea, changed it in
https://github.com/apache/nifi-minifi-cpp/pull/1352/commits/68929f499f8dc41248d06c564ca9ed21f84609a1#diff-2a26794f434d15c4c7911b91d7bf66a6e8069d49856da2b30c556439adb1b67cR67
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
