From bf0dfe07df214d3fc12a6f81f753e867cbee3790 Mon Sep 17 00:00:00 2001 From: Marten Gajda Date: Wed, 2 Sep 2020 00:33:57 +0200 Subject: [PATCH] Split ByDayFilter, implements #52 The original ByDayFilter was way to complex and hard to maintain. This commit breaks it into two much simpler classes. A part of the heavy lifting is now done in RecurrenceRule and should be subject to another refactoring. A nice side effect is a small performance improvement when filtering ByDay. --- .../org/dmfs/rfc5545/recur/ByDayFilter.java | 151 ++---------------- .../rfc5545/recur/ByDayPrefixedFilter.java | 93 +++++++++++ .../dmfs/rfc5545/recur/RecurrenceRule.java | 48 +++++- .../rfc5545/recur/RecurrenceRuleTest.java | 7 + 4 files changed, 160 insertions(+), 139 deletions(-) create mode 100644 src/main/java/org/dmfs/rfc5545/recur/ByDayPrefixedFilter.java diff --git a/src/main/java/org/dmfs/rfc5545/recur/ByDayFilter.java b/src/main/java/org/dmfs/rfc5545/recur/ByDayFilter.java index cec4961..49c38b6 100644 --- a/src/main/java/org/dmfs/rfc5545/recur/ByDayFilter.java +++ b/src/main/java/org/dmfs/rfc5545/recur/ByDayFilter.java @@ -1,5 +1,6 @@ /* - * Copyright (C) 2013 Marten Gajda + * Copyright 2020 Marten Gajda + * * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -12,18 +13,15 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. - * */ package org.dmfs.rfc5545.recur; import org.dmfs.rfc5545.Instance; +import org.dmfs.rfc5545.Weekday; import org.dmfs.rfc5545.calendarmetrics.CalendarMetrics; -import org.dmfs.rfc5545.recur.ByExpander.Scope; -import org.dmfs.rfc5545.recur.RecurrenceRule.Part; -import org.dmfs.rfc5545.recur.RecurrenceRule.WeekdayNum; -import java.util.List; +import java.util.Set; /** @@ -33,151 +31,30 @@ */ final class ByDayFilter implements ByFilter { - /** - * The scope of this rule. - */ - private final Scope mScope; - - /** - * A flag indicating that at least one of the week days has a positional prefix. - */ - private final boolean mHasPositions; - - /** - * An array of the weekdays and their positions in a packed form that allows us to find them with a simple integer comparison - */ - private final int[] mPackedDays; - - - /** - * Get a packed representation of a {@link WeekdayNum}. - * - * @param pos - * The position of the day or 0. - * @param day - * The number of the weekday. - * - * @return An int that contains the position and the weekday. - */ - private static int packWeekday(int pos, int day) - { - return (pos << 8) + day; - } - - - /** - * Get the weekday part of a packed day. - * - * @param packedDay - * The packed day int. - * - * @return The weekday. - */ - @SuppressWarnings("unused") - private static int unpackWeekday(int packedDay) - { - return packedDay & 0xff; - } - /** - * Get the positional part of a packed day. - * - * @param packedDay - * The packed day int. - * - * @return The position. + * The {@link CalendarMetrics} to use. */ - @SuppressWarnings("unused") - private static int unpackPos(int packedDay) - { - return packedDay >>> 8; - } + private final CalendarMetrics mCalendarMetrics; + private final Set mWeekdays; - /** - * The {@link CalendarMetrics} to use. - */ - final CalendarMetrics mCalendarMetrics; + private final Weekday[] mWeekdayArray = Weekday.values(); - public ByDayFilter(RecurrenceRule rule, CalendarMetrics calendarMetrics) + public ByDayFilter(CalendarMetrics calendarMetrics, Set weekdays) { mCalendarMetrics = calendarMetrics; - List byDay = rule.getByDayPart(); - - boolean hasByMonth = rule.hasPart(Part.BYMONTH); - Freq freq = rule.getFreq(); - - mScope = rule.hasPart( - Part.BYWEEKNO) || freq == Freq.WEEKLY ? (hasByMonth || freq == Freq.MONTHLY ? Scope.WEEKLY_AND_MONTHLY : Scope.WEEKLY) - : (hasByMonth || freq == Freq.MONTHLY ? Scope.MONTHLY : Scope.YEARLY); - - boolean hasPositions = false; - - mPackedDays = new int[byDay.size()]; - int count = 0; - for (WeekdayNum w : byDay) - { - if (w.pos != 0) - { - hasPositions = true; - } - mPackedDays[count] = packWeekday(w.pos, w.weekday.ordinal()); - ++count; - } - mHasPositions = hasPositions; + mWeekdays = weekdays; } @Override public boolean filter(long instance) { - // this is called if FREQ is <= DAILY or any of BYMONTHDAY or BYYEARDAY is present, so we don't have to filter by month here - int year = Instance.year(instance); - int month = Instance.month(instance); - int dayOfMonth = Instance.dayOfMonth(instance); - CalendarMetrics calendarMetrics = mCalendarMetrics; - int dayOfWeek = calendarMetrics.getDayOfWeek(year, month, dayOfMonth); - int[] packedDays = mPackedDays; - - if (!mHasPositions) - { - return StaticUtils.linearSearch(packedDays, packWeekday(0, dayOfWeek)) < 0; - } - else - { - switch (mScope) - { - case WEEKLY: - /* - * Note: if we're in a weekly scope we shouldn't be here. So we just ignore any days with positions. - */ - return StaticUtils.linearSearch(packedDays, packWeekday(0, dayOfWeek)) < 0; - - case WEEKLY_AND_MONTHLY: - case MONTHLY: - { - int nthDay = (dayOfMonth - 1) / 7 + 1; - int lastNthDay = (dayOfMonth - calendarMetrics.getDaysPerPackedMonth(year, month)) / 7 - 1; - return (nthDay <= 0 || StaticUtils.linearSearch(packedDays, packWeekday(nthDay, dayOfWeek)) < 0) - && (lastNthDay >= 0 || StaticUtils.linearSearch(packedDays, - packWeekday(lastNthDay, dayOfWeek)) < 0) - && StaticUtils.linearSearch(packedDays, packWeekday(0, dayOfWeek)) < 0; - } - case YEARLY: - { - int yearDay = calendarMetrics.getDayOfYear(year, month, dayOfMonth); - int nthDay = (yearDay - 1) / 7 + 1; - int lastNthDay = (yearDay - calendarMetrics.getDaysPerYear(year)) / 7 - 1; - return (nthDay <= 0 || StaticUtils.linearSearch(packedDays, packWeekday(nthDay, dayOfWeek)) < 0) - && (lastNthDay >= 0 || StaticUtils.linearSearch(packedDays, - packWeekday(lastNthDay, dayOfWeek)) < 0) - && StaticUtils.linearSearch(packedDays, packWeekday(0, dayOfWeek)) < 0; - } - default: - return false; - } - } + return !mWeekdays.contains(mWeekdayArray[mCalendarMetrics.getDayOfWeek( + Instance.year(instance), + Instance.month(instance), + Instance.dayOfMonth(instance))]); } } diff --git a/src/main/java/org/dmfs/rfc5545/recur/ByDayPrefixedFilter.java b/src/main/java/org/dmfs/rfc5545/recur/ByDayPrefixedFilter.java new file mode 100644 index 0000000..e97c1d4 --- /dev/null +++ b/src/main/java/org/dmfs/rfc5545/recur/ByDayPrefixedFilter.java @@ -0,0 +1,93 @@ +/* + * Copyright 2020 Marten Gajda + * + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.dmfs.rfc5545.recur; + +import org.dmfs.jems.function.BiFunction; +import org.dmfs.rfc5545.Instance; +import org.dmfs.rfc5545.Weekday; +import org.dmfs.rfc5545.calendarmetrics.CalendarMetrics; + +import java.util.Map; +import java.util.Set; + + +/** + * A filter that limits recurrence rules by day of week. + * + * @author Marten Gajda + */ +final class ByDayPrefixedFilter implements ByFilter +{ + public enum Scope + { + MONTH( + (instance, metrics) -> + (Instance.dayOfMonth(instance) - 1) / 7 + 1, + (instance, metrics) -> + (Instance.dayOfMonth(instance) - metrics.getDaysPerPackedMonth(Instance.year(instance), Instance.month(instance))) / 7 - 1), + YEAR( + (instance, metrics) -> + (metrics.getDayOfYear(Instance.year(instance), Instance.month(instance), Instance.dayOfMonth(instance)) - 1) / 7 + 1, + (instance, metrics) -> + (metrics.getDayOfYear(Instance.year(instance), Instance.month(instance), Instance.dayOfMonth(instance)) + - metrics.getDaysPerYear(Instance.year(instance))) / 7 - 1); + + private final BiFunction mNthDay; + private final BiFunction mNthLastDay; + + + Scope(BiFunction nthDay, BiFunction nthLastDay) + { + mNthDay = nthDay; + mNthLastDay = nthLastDay; + } + } + + + /** + * The {@link CalendarMetrics} to use. + */ + private final CalendarMetrics mCalendarMetrics; + private final Map> mPrefixedWeekDays; + private final Scope mScope; + private final Weekday[] mWeekdayArray = Weekday.values(); + + + public ByDayPrefixedFilter( + CalendarMetrics calendarMetrics, + Map> prefixedWeekDays, + Scope scope) + { + mCalendarMetrics = calendarMetrics; + mPrefixedWeekDays = prefixedWeekDays; + mScope = scope; + } + + + @Override + public boolean filter(long instance) + { + Set prefixes = mPrefixedWeekDays.get(mWeekdayArray[mCalendarMetrics.getDayOfWeek( + Instance.year(instance), + Instance.month(instance), + Instance.dayOfMonth(instance))]); + return prefixes == null + || (!prefixes.contains(mScope.mNthDay.value(instance, mCalendarMetrics)) + && !prefixes.contains(mScope.mNthLastDay.value(instance, mCalendarMetrics))); + } +} diff --git a/src/main/java/org/dmfs/rfc5545/recur/RecurrenceRule.java b/src/main/java/org/dmfs/rfc5545/recur/RecurrenceRule.java index 158cdbd..9c43b99 100644 --- a/src/main/java/org/dmfs/rfc5545/recur/RecurrenceRule.java +++ b/src/main/java/org/dmfs/rfc5545/recur/RecurrenceRule.java @@ -29,6 +29,7 @@ import java.util.EnumMap; import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -478,7 +479,50 @@ RuleIterator getExpander(RecurrenceRule rule, RuleIterator previous, CalendarMet @Override ByFilter getFilter(RecurrenceRule rule, CalendarMetrics calendarMetrics) throws UnsupportedOperationException { - return new ByDayFilter(rule, calendarMetrics); + Freq freq = rule.getFreq(); + // TODO: this method looks ugly and can use some improvement + // TODO: consider doing this separation at parsing time + Set nonPrefixed = EnumSet.noneOf(Weekday.class); + Map> prefixed = new EnumMap<>(Weekday.class); + + for (WeekdayNum wn : rule.getByDayPart()) + { + if (wn.pos == 0) + { + nonPrefixed.add(wn.weekday); + } + else + { + Set prefixes = prefixed.get(wn.weekday); + if (prefixes == null) // TODO use java 8 API, check Android support first + { + prefixes = new HashSet<>(); + prefixed.put(wn.weekday, prefixes); + } + prefixes.add(wn.pos); + } + } + + if (prefixed.isEmpty() + || freq != Freq.YEARLY && freq != Freq.MONTHLY + || freq == Freq.YEARLY && rule.hasPart(BYWEEKNO)) + { + return new ByDayFilter(calendarMetrics, nonPrefixed); + } + ByFilter baseFilter = new ByDayPrefixedFilter( + calendarMetrics, + prefixed, + freq == Freq.YEARLY && rule.getByPart(BYMONTH) == null ? + ByDayPrefixedFilter.Scope.YEAR : + ByDayPrefixedFilter.Scope.MONTH); + + if (nonPrefixed.isEmpty()) + { + return baseFilter; + } + + ByFilter nonPrefixFilter = new ByDayFilter(calendarMetrics, nonPrefixed); + return instance -> nonPrefixFilter.filter(instance) && baseFilter.filter(instance); } @@ -612,7 +656,7 @@ RuleIterator getExpander(RecurrenceRule rule, RuleIterator previous, CalendarMet @Override ByFilter getFilter(RecurrenceRule rule, CalendarMetrics calendarMetrics) throws UnsupportedOperationException { - return new ByDayFilter(rule, calendarMetrics); + return BYDAY.getFilter(rule, calendarMetrics); } diff --git a/src/test/java/org/dmfs/rfc5545/recur/RecurrenceRuleTest.java b/src/test/java/org/dmfs/rfc5545/recur/RecurrenceRuleTest.java index 1462f94..2bb19cb 100644 --- a/src/test/java/org/dmfs/rfc5545/recur/RecurrenceRuleTest.java +++ b/src/test/java/org/dmfs/rfc5545/recur/RecurrenceRuleTest.java @@ -25,6 +25,7 @@ import static org.dmfs.rfc5545.Weekday.MO; import static org.dmfs.rfc5545.Weekday.TH; import static org.dmfs.rfc5545.Weekday.TU; +import static org.dmfs.rfc5545.Weekday.WE; import static org.dmfs.rfc5545.hamcrest.RecurrenceRuleMatcher.are; import static org.dmfs.rfc5545.hamcrest.RecurrenceRuleMatcher.instances; import static org.dmfs.rfc5545.hamcrest.RecurrenceRuleMatcher.results; @@ -82,5 +83,11 @@ public void test() throws InvalidRecurrenceRuleException }, hasValue(hasToString("FREQ=MONTHLY;RSCALE=GREGORIAN;SKIP=FORWARD;COUNT=5")) ); + + assertThat(new RecurrenceRule("FREQ=MONTHLY;BYDAY=1MO,-1MO,WE"), + is(validRule(DateTime.parse("20200902"), + walking(), + instances(are(onWeekDay(MO, WE))), + startingWith("20200902", "20200907", "20200909", "20200916", "20200923", "20200928", "20200930", "20201005", "20201007")))); } }