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

LG-15493 | Enhance AAMVA maintenance windows spec #11753

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
123 changes: 60 additions & 63 deletions app/services/idv/aamva_state_maintenance_window.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,51 +5,57 @@ class AamvaStateMaintenanceWindow
# All AAMVA maintenance windows are expressed in 'ET' (LG-14028),
TZ = 'America/New_York'

Window = Data.define(:cron, :duration) do
def cron_val
Copy link
Member

Choose a reason for hiding this comment

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

Minor: I'd generally prefer we avoid the abbreviation since I don't think it's worth the potential readability hit to save two characters.

Suggested change
def cron_val
def cron_value

Copy link
Member Author

Choose a reason for hiding this comment

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

If I'm being honest, the issue here was more that I had no idea what to call this than a desire to keep it short. Is it a cron_value? A parsed_cron?

Time.use_zone(TZ) { Fugit.parse_cron(cron) }
end
end.freeze
Copy link
Member

Choose a reason for hiding this comment

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

I'm unsure, but does .freeze do anything for us here? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if there is any actual benefit in terms of memory, but it has the practical effect of keeping rubocop from yelling at me with a Style/MutableConstant complaint. 😇

Copy link
Contributor

Choose a reason for hiding this comment

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

another option, parse it in an initializer?

Suggested change
Window = Data.define(:cron, :duration) do
def cron_val
Time.use_zone(TZ) { Fugit.parse_cron(cron) }
end
end.freeze
Window = Data.define(:cron, :duration) do
def initialize(:cron, :duration)
super(
cron: Time.use_zone(TZ) { Fugit.parse_cron(cron) },
duration:
)
end
end.freeze

Copy link
Member Author

Choose a reason for hiding this comment

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

👏 This is the way.

I'm embarrassed to say that it somehow didn't occur to me that I could use an initializer here. But this feels like it solves the (minor) performance issue, and gets me away from bikeshedding names too.


MAINTENANCE_WINDOWS = {
'AL' => [
# First Monday of each month from 1 am – 7 am ET
{ cron: '0 1 * * Mon#1', duration_minutes: 6 * 60 },
Window.new(cron: '0 1 * * Mon#1', duration: 6.hours),
],
'CA' => [
# Daily, 4:00 - 5:30 am. ET.
{ cron: '0 4 * * *', duration_minutes: 90 },
Window.new(cron: '0 4 * * *', duration: 90.minutes),
# Monday, 1:00 - 1:45 am. ET
{ cron: '0 1 * * Mon', duration_minutes: 45 },
Window.new(cron: '0 1 * * Mon', duration: 45.minutes),
# Monday, 1:00 - 4:30 am. ET on 1st and 3rd Monday of month.
{ cron: '0 1 * * Mon#1', duration_minutes: 3.5 * 60 },
{ cron: '0 1 * * Mon#3', duration_minutes: 3.5 * 60 },
Window.new(cron: '0 1 * * Mon#1', duration: 3.5.hours),
Window.new(cron: '0 1 * * Mon#3', duration: 3.5.hours),
],
'CO' => [
# 02:00 - 08:00 AM ET on the first Tuesday of every month.
{ cron: '0 2 * * Tue#1', duration_minutes: 6 * 60 },
Window.new(cron: '0 2 * * Tue#1', duration: 6.hours),
],
'CT' => [
# Daily, 3:00 am. to 4:00 am. ET.
{ cron: '0 3 * * *', duration_minutes: 1 },
Window.new(cron: '0 3 * * *', duration: 1.hour),
Copy link
Member Author

Choose a reason for hiding this comment

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

An unexpected perk of moving to using a Duration is that doing the change semi-manually caused me to catch and fix this 1-minute vs. 1-hour mistake.

# Sunday 5:00 am. to 7:00 am. ET
{ cron: '0 6 * * Sun', duration_minutes: 2 * 60 },
Window.new(cron: '0 6 * * Sun', duration: 2.hours),
# second Sunday of month 4:00 am. to 8:00 am. ET
{ cron: '0 4 * * Sun#2', duration_minutes: 4 * 60 },
Window.new(cron: '0 4 * * Sun#2', duration: 4.hours),
],
'DC' => [
# Daily, Midnight to 6 am. ET.
{ cron: '0 0 * * *', duration_minutes: 6 * 60 },
Window.new(cron: '0 0 * * *', duration: 6.hours),
],
'DE' => [
# Daily, Midnight to 5 am. ET.
{ cron: '0 0 * * *', duration_minutes: 5 * 60 },
Window.new(cron: '0 0 * * *', duration: 5.hours),
],
'FL' => [
# Sunday 7:00 am. to 12:00 pm. ET
{ cron: '0 7 * * Sun', duration_minutes: 5 * 60 },
Window.new(cron: '0 7 * * Sun', duration: 5.hours),
],
'GA' => [
# Daily, 5:00 am. to 6:00 am. ET.
{ cron: '0 5 * * *', duration_minutes: 60 },
Window.new(cron: '0 5 * * *', duration: 1.hour),
],
'IA' => [
# "Daily, normally at 4:45 am. to 5:15 am ET."
{ cron: '45 4 * * *', duration_minutes: 30 },
Window.new(cron: '45 4 * * *', duration: 30.minutes),
# (Also "Sunday mornings but only seconds at a time.")
],
'ID' => [
Expand All @@ -59,150 +65,141 @@ class AamvaStateMaintenanceWindow
# I'm modeling this as _every_ Wednesday since we're really
# answering "Should we expect a maintenance window right now?",
# and we don't block the user from anything.
{ cron: '0 21 * * Wed', duration_minutes: 3 * 60 },
Window.new(cron: '0 21 * * Wed', duration: 3.hours),
],
'IL' => [
# Daily, 2:30 am. to 5:00 am. ET.
{ cron: '30 2 * * *', duration_minutes: 2.5 * 60 },
Window.new(cron: '30 2 * * *', duration: 2.5.hours),
],
'IN' => [
# Sunday 5:00 am. to 10:00 am. ET.
{ cron: '0 5 * * Sun', duration_minutes: 5 * 60 },
Window.new(cron: '0 5 * * Sun', duration: 5.hours),
],
'KS' => [
# Sunday: 7:00 am. to 1:00 pm. ET
{ cron: '0 7 * * Sun', duration_minutes: 6 * 60 },
Window.new(cron: '0 7 * * Sun', duration: 6.hours),
],
'KY' => [
# Daily maintenance from 2:35 am. to 6:40 am. ET
{ cron: '35 2 * * *', duration_minutes: 245 },
Window.new(cron: '35 2 * * *', duration: 245.minutes),
# "Monthly on Sunday, midnight to 10:00 am ET."
# (Okay, but _which_ Sunday?)
],
'MA' => [
# Daily 3:00 am. to 4:00 am. ET.
{ cron: '0 3 * * *', duration_minutes: 60 },
Window.new(cron: '0 3 * * *', duration: 1.hour),
# Saturday 10:00 pm. to Sunday 10:00 am
{ cron: '0 22 * * Sat', duration_minutes: 12 * 60 },
Window.new(cron: '0 22 * * Sat', duration: 12.hours),
# Sunday 2:00 am to 5:00 am ET
{ cron: '0 2 * * Sun', duration_minutes: 3 * 60 },
Window.new(cron: '0 2 * * Sun', duration: 3.hours),
# First Friday of each month: 12 to 6 am. ET.
{ cron: '0 0 * * Fri#1', duration_minutes: 6 * 60 },
Window.new(cron: '0 0 * * Fri#1', duration: 6.hours),
],
'MD' => [
# Sunday maintenance may occur from 6 am. to 10 am. ET.
{ cron: '0 6 * * Sun', duration_minutes: 4 * 60 },
Window.new(cron: '0 6 * * Sun', duration: 4.hours),
],
'MI' => [
# Daily maintenance from 9 pm. to 9:30 pm. ET.
{ cron: '0 21 * * *', duration_minutes: 30 },
Window.new(cron: '0 21 * * *', duration: 30.minutes),
],
'MO' => [
# Daily maintenance from 2 am. to 4:30 am. ...
{ cron: '0 2 * * *', duration_minutes: 2.5 * 60 },
Window.new(cron: '0 2 * * *', duration: 2.5.hours),
# ... from 6:30 am to 6:45 am ...
{ cron: '30 6 * * *', duration_minutes: 15 },
Window.new(cron: '30 6 * * *', duration: 15.minutes),
# ... and 8:30 am. to 8:35 am ET.
{ cron: '30 8 * * *', duration_minutes: 5 },
Window.new(cron: '30 8 * * *', duration: 5.minutes),
],
'MT' => [
# Third Saturday of odd numbered months from 12:00 am to 6:00 am ET
{ cron: '0 2 * /2 Sat#3', duration_minutes: 6 * 60 },
Window.new(cron: '0 2 * /2 Sat#3', duration: 6.hours),
],
'NC' => [
# Daily, Midnight to 7:00 am. ET.
{ cron: '0 0 * * *', duration_minutes: 7 * 60 },
Window.new(cron: '0 0 * * *', duration: 7.hours),
],
'ND' => [
# Wednesday around 7:30 pm to 7:35 pm ET
{ cron: '30 19 * * Wed', duration_minutes: 5 },
Window.new(cron: '30 19 * * Wed', duration: 5.minutes),
# 3rd Sunday of month, 5 minutes anytime between midnight and noon.
],
'NM' => [
# Sundays 8:00 am. to noon ET.
{ cron: '0 8 * * Sun', duration_minutes: 4 * 60 },
Window.new(cron: '0 8 * * Sun', duration: 4.hours),
],
'NV' => [
# Tuesdays to Sundays: 2:00 am. to 3:15 am. ET
{ cron: '0 2 * * Tue-Sun', duration_minutes: 1.25 * 60 },
Window.new(cron: '0 2 * * Tue-Sun', duration: 1.25.hours),
],
'NY' => [
# Sunday maintenance 8 pm. to 9 pm. ET.
{ cron: '0 20 * * Sun', duration_minutes: 60 },
Window.new(cron: '0 20 * * Sun', duration: 1.hour),
],
'OH' => [
# Daily 4:00 am. to 4:30 am. ET
{ cron: '0 4 * * *', duration_minutes: 30 },
Window.new(cron: '0 4 * * *', duration: 30.minutes),
],
'OR' => [
# Sunday 7:30 am. to 9:00 am. ET.
{ cron: '30 7 * * Sun', duration_minutes: 1.5 * 60 },
Window.new(cron: '30 7 * * Sun', duration: 1.5.hours),
],
'PA' => [
# Sunday 5:00 am. to 7:00 am. ET.
{ cron: '0 5 * * Sun', duration_minutes: 2 * 60 },
Window.new(cron: '0 5 * * Sun', duration: 2.hours),
],
'RI' => [
# Either 3rd or 4th Sunday of each month, 7:30 am. to 10:00 am. ET.
{ cron: '30 7 * * Sun#3', duration_minutes: 2.5 * 60 },
{ cron: '30 7 * * Sun#4', duration_minutes: 2.5 * 60 },
Window.new(cron: '30 7 * * Sun#3', duration: 2.5.hours),
Window.new(cron: '30 7 * * Sun#4', duration: 2.5.hours),
],
'SC' => [
# Sunday 6:00 pm. to 10:00 pm. ET.
{ cron: '0 18 * * Sun', duration_minutes: 4 * 60 },
Window.new(cron: '0 18 * * Sun', duration: 4.hours),
],
'TN' => [
# Last Sunday of every month from 11:00 pm Sunday to 2:00 am. Monday ET
{ cron: '0 23 * * Sun#last', duration_minutes: 3 * 60 },
Window.new(cron: '0 23 * * Sun#last', duration: 3.hours),
],
'TX' => [
# Saturday 9:00 pm. to Sunday 7:00 am. ET.
{ cron: '0 21 * * Sat', duration_minutes: 10 * 60 },
Window.new(cron: '0 21 * * Sat', duration: 10.hours),
],
'UT' => [
# 3rd Sunday of every month 1:00 am. to 9:00 am. ET
{ cron: '0 1 0 0 Sun#3', duration_minutes: 8 * 60 },
Window.new(cron: '0 1 * * Sun#3', duration: 8.hours),
],
'VA' => [
n1zyy marked this conversation as resolved.
Show resolved Hide resolved
# Daily 5:00 am. to 5:30 am. ET
{ cron: '0 5 * * *', duration_minutes: 30 },
Window.new(cron: '0 5 * * *', duration: 30.minutes),
# Sunday morning maintenance 3:00 am. to 5 am. ET.
{ cron: '0 3 * * Sun', duration_minutes: 2 * 60 },
Window.new(cron: '0 3 * * Sun', duration: 2.hours),
# "Might not respond for short spells, daily between 7 pm and 8:30 pm." (not modeling this)
],
'VT' => [
# Daily maintenance from midnight to 5 am. ET.
{ cron: '0 0 * * *', duration_minutes: 5 * 60 },
Window.new(cron: '0 0 * * *', duration: 5.hours),
],
'WA' => [
# Maintenance from Saturday 9:45 pm. to Sunday 8:15 am. ET.
{ cron: '45 21 * * Sat', duration_minutes: 10.5 * 60 },
Window.new(cron: '45 21 * * Sat', duration: 10.5.hours),
],
'WI' => [
# Downtime on Tuesday – Saturday typically between 3 – 4 am ET.
{ cron: '0 3 * * Tue-Sat', duration_minutes: 60 },
Window.new(cron: '0 3 * * Tue-Sat', duration: 1.hour),
# Downtime on Sunday from 6 – 10 am. ET.
{ cron: '0 6 * * Sun', duration_minutes: 4 * 60 },
Window.new(cron: '0 6 * * Sun', duration: 4.hours),
],
'WV' => [
# Sunday 6:00 am. to 6:20 am. ET
{ cron: '0 6 * * Sun', duration_minutes: 20 },
Window.new(cron: '0 6 * * Sun', duration: 20.minutes),
],
'WY' => [
# Daily, 2 am. to 5 am. ET.
{ cron: '0 2 * * *', duration_minutes: 3 * 60 },
Window.new(cron: '0 2 * * *', duration: 3.hours),
],
}.freeze

PARSED_MAINTENANCE_WINDOWS = MAINTENANCE_WINDOWS.transform_values do |windows|
Time.use_zone(TZ) do
windows.map do |window|
cron = Fugit.parse_cron(window[:cron])
{ cron: cron, duration_minutes: window[:duration_minutes] }
end
end
end.freeze
Copy link
Member Author

Choose a reason for hiding this comment

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

This clunkiness was for inline readability. Now that we have a Window Data object, though, I was able to implement a lil' cron_val method that performs this instead of building a whole separate data structure.

Academically, this lacks memoization since it's implemented on a frozen structure, which isn't great. IMHO this is acceptable because we're doing the parse_cron call after filtering down to the (US) state level, so worse case it's only a few calls, which seems to only take a millisecond or so in local testing.

Copy link
Member

Choose a reason for hiding this comment

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

Very optional, but you might be able to throw a lazy into the windows_for_state return value to get some optimization back, so that we only map up to the first .any? hit.

https://ruby-doc.org/core-2.7.0/Enumerator/Lazy.html


class << self
def in_maintenance_window?(state)
Time.use_zone(TZ) do
Expand All @@ -212,9 +209,9 @@ def in_maintenance_window?(state)

def windows_for_state(state)
Time.use_zone(TZ) do
PARSED_MAINTENANCE_WINDOWS.fetch(state, []).map do |window|
previous = window[:cron].previous_time.to_t
(previous..(previous + window[:duration_minutes].minutes))
MAINTENANCE_WINDOWS.fetch(state, []).map do |window|
previous = window.cron_val.previous_time.to_t
(previous..(previous + window.duration))
end
end
end
Expand Down
14 changes: 14 additions & 0 deletions spec/services/idv/aamva_state_maintenance_window_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,18 @@
end
end
end

describe 'MAINTENANCE_WINDOWS' do
described_class::MAINTENANCE_WINDOWS.each do |state, windows|
windows.each do |window|
it "consists of a valid cron expression and duration (#{state})" do
expect(window).to be_an(Idv::AamvaStateMaintenanceWindow::Window)

expect(window.cron_val).to be_a(Fugit::Cron)

expect(window.duration).to be_an(ActiveSupport::Duration)
end
end
end
end
end