Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

House keeping #117

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,8 @@ jobs:
run: cargo test --verbose
- name: Build docs
run: cargo doc --no-deps --verbose
- name: Check formatting
run: cargo fmt -- --check
- name: Check linting
run: cargo clippy --all-targets --all-features -- -D warnings

11 changes: 5 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![deny(rust_2018_idioms)]
#![deny(rustdoc::broken_intra_doc_links)]

#![allow(clippy::needless_doctest_main)]
//! A cron expression parser and schedule explorer
//! # Example
Expand Down Expand Up @@ -37,12 +36,12 @@
/// Error types used by this crate.
pub mod error;

mod schedule;
mod time_unit;
mod ordinal;
mod specifier;
mod queries;
mod parsing;
mod queries;
mod schedule;
mod specifier;
mod time_unit;

pub use crate::schedule::{Schedule, ScheduleIterator, OwnedScheduleIterator};
pub use crate::schedule::{OwnedScheduleIterator, Schedule, ScheduleIterator};
pub use crate::time_unit::TimeUnitSpec;
2 changes: 1 addition & 1 deletion src/ordinal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ pub type Ordinal = u32;
// It should either be a BTreeSet of ordinals or an `All` option to save space.
// `All` can iterate from inclusive_min to inclusive_max and answer membership
// queries
pub type OrdinalSet = BTreeSet<Ordinal>;
pub type OrdinalSet = BTreeSet<Ordinal>;
24 changes: 17 additions & 7 deletions src/parsing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ use std::convert::TryFrom;
use std::str::{self, FromStr};

use crate::error::{Error, ErrorKind};
use crate::schedule::{ScheduleFields, Schedule};
use crate::ordinal::*;
use crate::schedule::{Schedule, ScheduleFields};
use crate::specifier::*;
use crate::time_unit::*;
use crate::ordinal::*;

impl FromStr for Schedule {
type Err = Error;
Expand Down Expand Up @@ -52,10 +52,12 @@ where
T: TimeUnitField,
{
fn from_field(field: Field) -> Result<T, Error> {
if field.specifiers.len() == 1 &&
field.specifiers.get(0).unwrap() == &RootSpecifier::from(Specifier::All)
{ return Ok(T::all()); }
let mut ordinals = OrdinalSet::new();
if field.specifiers.len() == 1
&& field.specifiers.get(0).unwrap() == &RootSpecifier::from(Specifier::All)
{
return Ok(T::all());
}
let mut ordinals = OrdinalSet::new();
for specifier in field.specifiers {
let specifier_ordinals: OrdinalSet = T::ordinals_from_root_specifier(&specifier)?;
for ordinal in specifier_ordinals {
Expand Down Expand Up @@ -255,7 +257,15 @@ fn longhand(i: &str) -> IResult<&str, ScheduleFields> {
let months = map_res(field, Months::from_field);
let days_of_week = map_res(field_with_any, DaysOfWeek::from_field);
let years = opt(map_res(field, Years::from_field));
let fields = tuple((seconds, minutes, hours, days_of_month, months, days_of_week, years));
let fields = tuple((
seconds,
minutes,
hours,
days_of_month,
months,
days_of_week,
years,
));

map(
terminated(fields, eof),
Expand Down
2 changes: 1 addition & 1 deletion src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,4 @@ where
pub fn reset_second(&mut self) {
self.first_second = false;
}
}
}
146 changes: 107 additions & 39 deletions src/schedule.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@

use chrono::offset::TimeZone;
use chrono::{DateTime, Datelike, Timelike, Utc};
use std::ops::Bound::{Included, Unbounded};
use std::fmt::{Display, Formatter, Result as FmtResult};
use std::ops::Bound::{Included, Unbounded};

use crate::time_unit::*;
use crate::ordinal::*;
use crate::queries::*;
use crate::time_unit::*;

impl From<Schedule> for String {
fn from(schedule: Schedule) -> String {
Expand All @@ -21,14 +20,8 @@ pub struct Schedule {
}

impl Schedule {
pub(crate) fn new(
source: String,
fields: ScheduleFields,
) -> Schedule {
Schedule {
source,
fields,
}
pub(crate) fn new(source: String, fields: ScheduleFields) -> Schedule {
Schedule { source, fields }
}

fn next_after<Z>(&self, after: &DateTime<Z>) -> Option<DateTime<Z>>
Expand All @@ -50,7 +43,12 @@ impl Schedule {
let month_range = (Included(month_start), Included(Months::inclusive_max()));
for month in self.fields.months.ordinals().range(month_range).cloned() {
let day_of_month_start = query.day_of_month_lower_bound();
if !self.fields.days_of_month.ordinals().contains(&day_of_month_start) {
if !self
.fields
.days_of_month
.ordinals()
.contains(&day_of_month_start)
{
query.reset_day_of_month();
}
let day_of_month_end = days_in_month(month, year);
Expand Down Expand Up @@ -85,11 +83,20 @@ impl Schedule {
let second_range =
(Included(second_start), Included(Seconds::inclusive_max()));

for second in self.fields.seconds.ordinals().range(second_range).cloned() {
for second in
self.fields.seconds.ordinals().range(second_range).cloned()
{
let timezone = after.timezone();
let candidate = if let Some(candidate) = timezone
.ymd(year as i32, month, day_of_month)
.and_hms_opt(hour, minute, second)
.with_ymd_and_hms(
year as i32,
month,
day_of_month,
hour,
minute,
second,
)
.single()
{
candidate
} else {
Expand Down Expand Up @@ -139,9 +146,21 @@ impl Schedule {
}
let month_range = (Included(Months::inclusive_min()), Included(month_start));

for month in self.fields.months.ordinals().range(month_range).rev().cloned() {
for month in self
.fields
.months
.ordinals()
.range(month_range)
.rev()
.cloned()
{
let day_of_month_end = query.day_of_month_upper_bound();
if !self.fields.days_of_month.ordinals().contains(&day_of_month_end) {
if !self
.fields
.days_of_month
.ordinals()
.contains(&day_of_month_end)
{
query.reset_day_of_month();
}

Expand All @@ -166,28 +185,57 @@ impl Schedule {
}
let hour_range = (Included(Hours::inclusive_min()), Included(hour_start));

for hour in self.fields.hours.ordinals().range(hour_range).rev().cloned() {
for hour in self
.fields
.hours
.ordinals()
.range(hour_range)
.rev()
.cloned()
{
let minute_start = query.minute_upper_bound();
if !self.fields.minutes.ordinals().contains(&minute_start) {
query.reset_minute();
}
let minute_range =
(Included(Minutes::inclusive_min()), Included(minute_start));

for minute in self.fields.minutes.ordinals().range(minute_range).rev().cloned() {
for minute in self
.fields
.minutes
.ordinals()
.range(minute_range)
.rev()
.cloned()
{
let second_start = query.second_upper_bound();
if !self.fields.seconds.ordinals().contains(&second_start) {
query.reset_second();
}
let second_range =
(Included(Seconds::inclusive_min()), Included(second_start));

for second in self.fields.seconds.ordinals().range(second_range).rev().cloned()
for second in self
.fields
.seconds
.ordinals()
.range(second_range)
.rev()
.cloned()
{
let timezone = before.timezone();
let candidate = if let Some(candidate) = timezone
.ymd(year as i32, month, day_of_month)
.and_hms_opt(hour, minute, second)
// .ymd(year as i32, month, day_of_month)
// .and_hms_opt(hour, minute, second)
.with_ymd_and_hms(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this might interfere with #115

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zslayton #112 #115 #116 all look like quality bug fixes.

I think it would be best if those were merged first then I will rebase this PR.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two of these have been merged, #115 (the most substantive change) is still pending. #123 addressed the current clippy and rustfmt issues, but did not add those steps to the CI. If you're interested in adding the CI steps as a fresh PR, that might be the quickest path forward.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, sorry I missed that this PR included the same fixes for fmt and clippy as I made in #123, sorry about that! But I didn't add any CI checks (which I should've) so this PR and changes still looks like a good one to merge!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to rebase this today!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alextopher #129 just got merged. In case you are currently working on the rebase, I hope this message reaches you before another rebase causes you extra work.

year as i32,
month,
day_of_month,
hour,
minute,
second,
)
.single()
{
candidate
} else {
Expand Down Expand Up @@ -248,13 +296,19 @@ impl Schedule {
where
Z: TimeZone,
{
self.fields.years.includes(date_time.year() as Ordinal) &&
self.fields.months.includes(date_time.month() as Ordinal) &&
self.fields.days_of_week.includes(date_time.weekday().number_from_sunday()) &&
self.fields.days_of_month.includes(date_time.day() as Ordinal) &&
self.fields.hours.includes(date_time.hour() as Ordinal) &&
self.fields.minutes.includes(date_time.minute() as Ordinal) &&
self.fields.seconds.includes(date_time.second() as Ordinal)
self.fields.years.includes(date_time.year() as Ordinal)
&& self.fields.months.includes(date_time.month() as Ordinal)
&& self
.fields
.days_of_week
.includes(date_time.weekday().number_from_sunday())
&& self
.fields
.days_of_month
.includes(date_time.day() as Ordinal)
&& self.fields.hours.includes(date_time.hour() as Ordinal)
&& self.fields.minutes.includes(date_time.minute() as Ordinal)
&& self.fields.seconds.includes(date_time.second() as Ordinal)
}

/// Returns a [TimeUnitSpec] describing the years included in this [Schedule].
Expand Down Expand Up @@ -398,18 +452,30 @@ where
}

/// A `ScheduleIterator` with a static lifetime.
pub struct OwnedScheduleIterator<Z> where Z: TimeZone {
pub struct OwnedScheduleIterator<Z>
where
Z: TimeZone,
{
schedule: Schedule,
previous_datetime: Option<DateTime<Z>>
previous_datetime: Option<DateTime<Z>>,
}

impl<Z> OwnedScheduleIterator<Z> where Z: TimeZone {
impl<Z> OwnedScheduleIterator<Z>
where
Z: TimeZone,
{
pub fn new(schedule: Schedule, starting_datetime: DateTime<Z>) -> Self {
Self { schedule, previous_datetime: Some(starting_datetime) }
Self {
schedule,
previous_datetime: Some(starting_datetime),
}
}
}

impl<Z> Iterator for OwnedScheduleIterator<Z> where Z: TimeZone {
impl<Z> Iterator for OwnedScheduleIterator<Z>
where
Z: TimeZone,
{
type Item = DateTime<Z>;

fn next(&mut self) -> Option<DateTime<Z>> {
Expand Down Expand Up @@ -457,7 +523,7 @@ fn days_in_month(month: Ordinal, year: Ordinal) -> u32 {
#[cfg(test)]
mod test {
use super::*;
use std::str::{FromStr};
use std::str::FromStr;

#[test]
fn test_next_and_prev_from() {
Expand Down Expand Up @@ -614,8 +680,9 @@ mod test {

let schedule_tz: Tz = "Europe/London".parse().unwrap();
let dt = schedule_tz
.ymd(2019, 10, 27)
.and_hms(0, 3, 29)
.with_ymd_and_hms(2019, 10, 27, 0, 3, 29)
.single()
.unwrap()
.checked_add_signed(chrono::Duration::hours(1)) // puts it in the middle of the DST transition
.unwrap();
let schedule = Schedule::from_str("* * * * * Sat,Sun *").unwrap();
Expand All @@ -630,8 +697,9 @@ mod test {

let schedule_tz: Tz = "Europe/London".parse().unwrap();
let dt = schedule_tz
.ymd(2019, 10, 27)
.and_hms(0, 3, 29)
.with_ymd_and_hms(2019, 10, 27, 0, 3, 29)
.single()
.unwrap()
.checked_add_signed(chrono::Duration::hours(1)) // puts it in the middle of the DST transition
.unwrap();
let schedule = Schedule::from_str("* * * * * Sat,Sun *").unwrap();
Expand Down
2 changes: 1 addition & 1 deletion src/specifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ impl From<Specifier> for RootSpecifier {
fn from(specifier: Specifier) -> Self {
Self::Specifier(specifier)
}
}
}
14 changes: 7 additions & 7 deletions src/time_unit/days_of_month.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
use crate::ordinal::{Ordinal, OrdinalSet};
use crate::time_unit::TimeUnitField;
use std::borrow::Cow;
use once_cell::sync::Lazy;
use std::borrow::Cow;

static ALL: Lazy<OrdinalSet> = Lazy::new(|| { DaysOfMonth::supported_ordinals() });
static ALL: Lazy<OrdinalSet> = Lazy::new(DaysOfMonth::supported_ordinals);

#[derive(Clone, Debug, Eq)]
pub struct DaysOfMonth{
ordinals: Option<OrdinalSet>
pub struct DaysOfMonth {
ordinals: Option<OrdinalSet>,
}

impl TimeUnitField for DaysOfMonth {
fn from_optional_ordinal_set(ordinal_set: Option<OrdinalSet>) -> Self {
DaysOfMonth {
ordinals: ordinal_set
ordinals: ordinal_set,
}
}
fn name() -> Cow<'static, str> {
Expand All @@ -28,7 +28,7 @@ impl TimeUnitField for DaysOfMonth {
fn ordinals(&self) -> &OrdinalSet {
match &self.ordinals {
Some(ordinal_set) => ordinal_set,
None => &ALL
None => &ALL,
}
}
}
Expand All @@ -37,4 +37,4 @@ impl PartialEq for DaysOfMonth {
fn eq(&self, other: &DaysOfMonth) -> bool {
self.ordinals() == other.ordinals()
}
}
}
Loading