GitHub/Anki
1
0
Fork 0
mirror of https://github.com/ankitects/anki.git synced 2025-09-18 14:02:21 -04:00
Code Issues Projects Releases Packages 1 Wiki Activity Actions

tidy up UTC offset handling/timing calculations

Browse source 
- use the TimestampSecs newtype instead of raw i64s
- use FixedOffset instead of a minutes_west offset
- check localOffset each time the timing is calculated, and set it
if it's stale - even for v1.
- check for and fix missing rollover when calculating timing
- stop explicitly passing localOffset in the sync/start call
This commit is contained in:
Damien Elmes 2021-01-12 21:29:22 +10:00
parent dfbb415db3
commit fbd91b22f5
16 changed files with 139 additions and 156 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
pylib/anki/collection.py
View file

@ -135,14 +135,6 @@ class Collection:
self._loadScheduler()
# the sync code uses this to send the local timezone to AnkiWeb
def localOffset(self) -> Optional[int]:
"Minutes west of UTC. Only applies to V2 scheduler."
if isinstance(self.sched, V1Scheduler):
return None
else:
return self.backend.local_minutes_west(intTime())
# DB-related
##########################################################################

2
pylib/rsbridge/lib.rs
View file

@ -40,13 +40,11 @@ fn want_release_gil(method: u32) -> bool {
| BackendMethod::RenderExistingCard
| BackendMethod::RenderUncommittedCard
| BackendMethod::StripAVTags
| BackendMethod::LocalMinutesWest
| BackendMethod::SchedTimingToday
| BackendMethod::AddOrUpdateDeckLegacy
| BackendMethod::NewDeckLegacy
| BackendMethod::NewDeckConfigLegacy
| BackendMethod::GetStockNotetypeLegacy
| BackendMethod::SetLocalMinutesWest
| BackendMethod::StudiedToday
| BackendMethod::TranslateString
| BackendMethod::FormatTimespan

2
rslib/backend.proto
View file

@ -95,8 +95,6 @@ service BackendService {
// scheduling
rpc LocalMinutesWest(Int64) returns (Int32);
rpc SetLocalMinutesWest(Int32) returns (Empty);
rpc SchedTimingToday(Empty) returns (SchedTimingTodayOut);
rpc StudiedToday(Empty) returns (String);
rpc StudiedTodayMessage(StudiedTodayMessageIn) returns (String);

0
rslib/src/backend/http_sync_server.rs Normal file
View file

15
rslib/src/backend/mod.rs
View file

@ -32,7 +32,6 @@ use crate::{
all_stock_notetypes, CardTemplateSchema11, NoteType, NoteTypeID, NoteTypeSchema11,
RenderCardOutput,
},
sched::cutoff::local_minutes_west_for_stamp,
sched::new::NewCardSortOrder,
sched::timespan::{answer_button_time, time_span},
search::{
@ -533,20 +532,6 @@ impl BackendService for Backend {
})
}
fn local_minutes_west(&self, input: pb::Int64) -> BackendResult<pb::Int32> {
Ok(pb::Int32 {
val: local_minutes_west_for_stamp(input.val),
})
}
fn set_local_minutes_west(&self, input: pb::Int32) -> BackendResult<Empty> {
self.with_col(|col| {
col.transact(None, |col| {
col.set_local_mins_west(input.val).map(Into::into)
})
})
}
/// Fetch data from DB and return rendered string.
fn studied_today(&self, _input: pb::Empty) -> BackendResult<pb::String> {
self.with_col(|col| col.studied_today().map(Into::into))

8
rslib/src/config.rs
View file

@ -141,11 +141,11 @@ impl Collection {
.unwrap_or(DeckID(1))
}
pub(crate) fn get_creation_mins_west(&self) -> Option<i32> {
pub(crate) fn get_creation_utc_offset(&self) -> Option<i32> {
self.get_config_optional(ConfigKey::CreationOffset)
}
pub(crate) fn set_creation_mins_west(&self, mins: Option<i32>) -> Result<()> {
pub(crate) fn set_creation_utc_offset(&self, mins: Option<i32>) -> Result<()> {
if let Some(mins) = mins {
self.set_config(ConfigKey::CreationOffset, &mins)
} else {
@ -153,11 +153,11 @@ impl Collection {
}
}
pub(crate) fn get_local_mins_west(&self) -> Option<i32> {
pub(crate) fn get_configured_utc_offset(&self) -> Option<i32> {
self.get_config_optional(ConfigKey::LocalOffset)
}
pub(crate) fn set_local_mins_west(&self, mins: i32) -> Result<()> {
pub(crate) fn set_configured_utc_offset(&self, mins: i32) -> Result<()> {
self.set_config(ConfigKey::LocalOffset, &mins)
}

13
rslib/src/preferences.rs
View file

@ -9,7 +9,6 @@ use crate::{
collection::Collection,
err::Result,
sched::cutoff::local_minutes_west_for_stamp,
timestamp::TimestampSecs,
};
impl Collection {
@ -43,7 +42,7 @@ impl Collection {
show_remaining_due_counts: self.get_show_due_counts(),
show_intervals_on_buttons: self.get_show_intervals_above_buttons(),
time_limit_secs: self.get_answer_time_limit_secs(),
new_timezone: self.get_creation_mins_west().is_some(),
new_timezone: self.get_creation_utc_offset().is_some(),
day_learn_first: self.get_day_learn_first(),
})
}
@ -73,15 +72,11 @@ impl Collection {
}
if s.new_timezone {
if self.get_creation_mins_west().is_none() {
self.set_creation_mins_west(Some(local_minutes_west_for_stamp(created.0)))?;
if self.get_creation_utc_offset().is_none() {
self.set_creation_utc_offset(Some(local_minutes_west_for_stamp(created.0)))?;
}
} else {
self.set_creation_mins_west(None)?;
}
if s.scheduler_version != 1 {
self.set_local_mins_west(local_minutes_west_for_stamp(TimestampSecs::now().0))?;
self.set_creation_utc_offset(None)?;
}
// fixme: currently scheduler change unhandled

121
rslib/src/sched/cutoff.rs
View file

@ -1,7 +1,7 @@
// Copyright: Ankitects Pty Ltd and contributors
// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
use crate::timestamp::TimestampSecs;
use crate::prelude::*;
use chrono::{Date, Duration, FixedOffset, Local, TimeZone, Timelike};
#[derive(Debug, PartialEq, Clone, Copy)]
@ -13,24 +13,21 @@ pub struct SchedTimingToday {
}
/// Timing information for the current day.
/// - created_secs is a UNIX timestamp of the collection creation time
/// - created_mins_west is the offset west of UTC at the time of creation
/// (eg UTC+10 hours is -600)
/// - now_secs is a timestamp of the current time
/// - now_mins_west is the current offset west of UTC
/// - creation_secs is a UNIX timestamp of the collection creation time
/// - creation_utc_offset is the UTC offset at collection creation time
/// - current_secs is a timestamp of the current time
/// - current_utc_offset is the current UTC offset
/// - rollover_hour is the hour of the day the rollover happens (eg 4 for 4am)
pub fn sched_timing_today_v2_new(
created_secs: i64,
created_mins_west: i32,
now_secs: i64,
now_mins_west: i32,
creation_secs: TimestampSecs,
creation_utc_offset: FixedOffset,
current_secs: TimestampSecs,
current_utc_offset: FixedOffset,
rollover_hour: u8,
) -> SchedTimingToday {
// get date(times) based on timezone offsets
let created_date = fixed_offset_from_minutes(created_mins_west)
.timestamp(created_secs, 0)
.date();
let now_datetime = fixed_offset_from_minutes(now_mins_west).timestamp(now_secs, 0);
let created_date = creation_secs.datetime(creation_utc_offset).date();
let now_datetime = current_secs.datetime(current_utc_offset);
let today = now_datetime.date();
// rollover
@ -118,9 +115,9 @@ fn v1_creation_date_adjusted_to_hour_inner(crt: i64, hour: u8, offset: FixedOffs
.timestamp()
}
fn sched_timing_today_v1(crt: i64, now: i64) -> SchedTimingToday {
let days_elapsed = (now - crt) / 86_400;
let next_day_at = crt + (days_elapsed + 1) * 86_400;
fn sched_timing_today_v1(crt: TimestampSecs, now: TimestampSecs) -> SchedTimingToday {
let days_elapsed = (now.0 - crt.0) / 86_400;
let next_day_at = crt.0 + (days_elapsed + 1) * 86_400;
SchedTimingToday {
days_elapsed: days_elapsed as u32,
next_day_at,
@ -128,26 +125,24 @@ fn sched_timing_today_v1(crt: i64, now: i64) -> SchedTimingToday {
}
fn sched_timing_today_v2_legacy(
crt: i64,
crt: TimestampSecs,
rollover: u8,
now: i64,
mins_west: i32,
now: TimestampSecs,
current_utc_offset: FixedOffset,
) -> SchedTimingToday {
let offset = fixed_offset_from_minutes(mins_west);
let crt_at_rollover = offset
.timestamp(crt, 0)
let crt_at_rollover = crt
.datetime(current_utc_offset)
.date()
.and_hms(rollover as u32, 0, 0)
.timestamp();
let days_elapsed = (now - crt_at_rollover) / 86_400;
let days_elapsed = (now.0 - crt_at_rollover) / 86_400;
let mut next_day_at = offset
.timestamp(now, 0)
let mut next_day_at = now
.datetime(current_utc_offset)
.date()
.and_hms(rollover as u32, 0, 0)
.timestamp();
if next_day_at < now {
if next_day_at < now.0 {
next_day_at += 86_400;
}
@ -159,27 +154,33 @@ fn sched_timing_today_v2_legacy(
// ----------------------------------
/// Based on provided input, get timing info from the relevant function.
/// Decide which scheduler timing to use based on the provided input,
/// and return the relevant timing info.
pub(crate) fn sched_timing_today(
created_secs: TimestampSecs,
now_secs: TimestampSecs,
created_mins_west: Option<i32>,
now_mins_west: Option<i32>,
creation_secs: TimestampSecs,
current_secs: TimestampSecs,
creation_utc_offset: Option<FixedOffset>,
current_utc_offset: FixedOffset,
rollover_hour: Option<u8>,
) -> SchedTimingToday {
let now_west = now_mins_west.unwrap_or_else(|| local_minutes_west_for_stamp(now_secs.0));
match (rollover_hour, created_mins_west) {
match (rollover_hour, creation_utc_offset) {
(None, _) => {
// if rollover unset, v1 scheduler
sched_timing_today_v1(created_secs.0, now_secs.0)
sched_timing_today_v1(creation_secs, current_secs)
}
(Some(roll), None) => {
(Some(rollover), None) => {
// if creationOffset unset, v2 scheduler with legacy cutoff handling
sched_timing_today_v2_legacy(created_secs.0, roll, now_secs.0, now_west)
sched_timing_today_v2_legacy(creation_secs, rollover, current_secs, current_utc_offset)
}
(Some(roll), Some(crt_west)) => {
(Some(rollover), Some(creation_utc_offset)) => {
// v2 scheduler, new cutoff handling
sched_timing_today_v2_new(created_secs.0, crt_west, now_secs.0, now_west, roll)
sched_timing_today_v2_new(
creation_secs,
creation_utc_offset,
current_secs,
current_utc_offset,
rollover,
)
}
}
}
@ -192,6 +193,10 @@ mod test {
// static timezone for tests
const AEST_MINS_WEST: i32 = -600;
fn aest_offset() -> FixedOffset {
FixedOffset::west(AEST_MINS_WEST * 60)
}
#[test]
fn fixed_offset() {
let offset = fixed_offset_from_minutes(AEST_MINS_WEST);
@ -200,6 +205,10 @@ mod test {
// helper
fn elap(start: i64, end: i64, start_west: i32, end_west: i32, rollhour: u8) -> u32 {
let start = TimestampSecs(start);
let end = TimestampSecs(end);
let start_west = FixedOffset::west(start_west * 60);
let end_west = FixedOffset::west(end_west * 60);
let today = sched_timing_today_v2_new(start, start_west, end, end_west, rollhour);
today.days_elapsed
}
@ -312,10 +321,10 @@ mod test {
let now = Local.ymd(2019, 1, 3).and_hms(2, 0, 0);
let next_day_at = Local.ymd(2019, 1, 3).and_hms(rollhour, 0, 0);
let today = sched_timing_today_v2_new(
crt.timestamp(),
crt.offset().utc_minus_local() / 60,
now.timestamp(),
now.offset().utc_minus_local() / 60,
TimestampSecs(crt.timestamp()),
*crt.offset(),
TimestampSecs(now.timestamp()),
*now.offset(),
rollhour as u8,
);
assert_eq!(today.next_day_at, next_day_at.timestamp());
@ -324,10 +333,10 @@ mod test {
let now = Local.ymd(2019, 1, 3).and_hms(rollhour, 0, 0);
let next_day_at = Local.ymd(2019, 1, 4).and_hms(rollhour, 0, 0);
let today = sched_timing_today_v2_new(
crt.timestamp(),
crt.offset().utc_minus_local() / 60,
now.timestamp(),
now.offset().utc_minus_local() / 60,
TimestampSecs(crt.timestamp()),
*crt.offset(),
TimestampSecs(now.timestamp()),
*now.offset(),
rollhour as u8,
);
assert_eq!(today.next_day_at, next_day_at.timestamp());
@ -336,10 +345,10 @@ mod test {
let now = Local.ymd(2019, 1, 3).and_hms(rollhour + 3, 0, 0);
let next_day_at = Local.ymd(2019, 1, 4).and_hms(rollhour, 0, 0);
let today = sched_timing_today_v2_new(
crt.timestamp(),
crt.offset().utc_minus_local() / 60,
now.timestamp(),
now.offset().utc_minus_local() / 60,
TimestampSecs(crt.timestamp()),
*crt.offset(),
TimestampSecs(now.timestamp()),
*now.offset(),
rollhour as u8,
);
assert_eq!(today.next_day_at, next_day_at.timestamp());
@ -347,10 +356,10 @@ mod test {
#[test]
fn legacy_timing() {
let now = 1584491078;
let now = TimestampSecs(1584491078);
assert_eq!(
sched_timing_today_v1(1575226800, now),
sched_timing_today_v1(TimestampSecs(1575226800), now),
SchedTimingToday {
days_elapsed: 107,
next_day_at: 1584558000
@ -358,7 +367,7 @@ mod test {
);
assert_eq!(
sched_timing_today_v2_legacy(1533564000, 0, now, AEST_MINS_WEST),
sched_timing_today_v2_legacy(TimestampSecs(1533564000), 0, now, aest_offset()),
SchedTimingToday {
days_elapsed: 589,
next_day_at: 1584540000
@ -366,7 +375,7 @@ mod test {
);
assert_eq!(
sched_timing_today_v2_legacy(1524038400, 4, now, AEST_MINS_WEST),
sched_timing_today_v2_legacy(TimestampSecs(1524038400), 4, now, aest_offset()),
SchedTimingToday {
days_elapsed: 700,
next_day_at: 1584554400

69
rslib/src/sched/mod.rs
View file

@ -1,9 +1,7 @@
// Copyright: Ankitects Pty Ltd and contributors
// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
use crate::{
collection::Collection, config::SchedulerVersion, err::Result, timestamp::TimestampSecs,
};
use crate::{collection::Collection, config::SchedulerVersion, err::Result, prelude::*};
pub mod bury_and_suspend;
pub(crate) mod congrats;
@ -15,8 +13,8 @@ pub mod timespan;
use chrono::FixedOffset;
use cutoff::{
fixed_offset_from_minutes, local_minutes_west_for_stamp, sched_timing_today,
v1_creation_date_adjusted_to_hour, v1_rollover_from_creation_stamp, SchedTimingToday,
sched_timing_today, v1_creation_date_adjusted_to_hour, v1_rollover_from_creation_stamp,
SchedTimingToday,
};
impl Collection {
@ -29,37 +27,62 @@ impl Collection {
}
pub(crate) fn timing_for_timestamp(&self, now: TimestampSecs) -> Result<SchedTimingToday> {
let local_offset = if self.server {
self.get_local_mins_west()
} else {
None
};
let current_utc_offset = self.local_utc_offset_for_user()?;
let rollover_hour = match self.sched_ver() {
SchedulerVersion::V1 => None,
SchedulerVersion::V2 => self.get_v2_rollover().or(Some(4)),
SchedulerVersion::V2 => {
let configured_rollover = self.get_v2_rollover();
match configured_rollover {
None => {
// an older Anki version failed to set this; correct
// the issue
self.set_v2_rollover(4)?;
Some(4)
}
val => val,
}
}
};
Ok(sched_timing_today(
self.storage.creation_stamp()?,
now,
self.get_creation_mins_west(),
local_offset,
self.creation_utc_offset(),
current_utc_offset,
rollover_hour,
))
}
/// Get the local timezone.
/// We could use this to simplify timing_for_timestamp() in the future
pub(crate) fn local_offset(&self) -> FixedOffset {
let local_mins_west = if self.server {
self.get_local_mins_west()
/// In the client case, return the current local timezone offset,
/// ensuring the config reflects the current value.
/// In the server case, return the value set in the config, and
/// fall back on UTC if it's missing/invalid.
pub(crate) fn local_utc_offset_for_user(&self) -> Result<FixedOffset> {
let config_tz = self
.get_configured_utc_offset()
.and_then(|v| FixedOffset::west_opt(v * 60))
.unwrap_or_else(|| FixedOffset::west(0));
let local_tz = TimestampSecs::now().local_utc_offset();
Ok(if self.server {
config_tz
} else {
None
};
let local_mins_west =
local_mins_west.unwrap_or_else(|| local_minutes_west_for_stamp(TimestampSecs::now().0));
fixed_offset_from_minutes(local_mins_west)
// if the timezone has changed, update the config
if config_tz != local_tz {
self.set_configured_utc_offset(local_tz.utc_minus_local() / 60)?;
}
local_tz
})
}
/// Return the timezone offset at collection creation time. This should
/// only be set when the V2 scheduler is active and the new timezone
/// code is enabled.
fn creation_utc_offset(&self) -> Option<FixedOffset> {
self.get_creation_utc_offset()
.and_then(|v| FixedOffset::west_opt(v * 60))
}
pub fn rollover_for_current_scheduler(&self) -> Result<u8> {

10
rslib/src/stats/card.rs
View file

@ -58,7 +58,7 @@ struct RevlogText {
impl Collection {
pub fn card_stats(&mut self, cid: CardID) -> Result<String> {
let stats = self.gather_card_stats(cid)?;
Ok(self.card_stats_to_string(stats))
self.card_stats_to_string(stats)
}
fn gather_card_stats(&mut self, cid: CardID) -> Result<CardStats> {
@ -126,8 +126,8 @@ impl Collection {
})
}
fn card_stats_to_string(&self, cs: CardStats) -> String {
let offset = self.local_offset();
fn card_stats_to_string(&self, cs: CardStats) -> Result<String> {
let offset = self.local_utc_offset_for_user()?;
let i18n = &self.i18n;
let mut stats = vec![(
@ -216,13 +216,13 @@ impl Collection {
taken_secs: i18n.tr(TR::CardStatsReviewLogTimeTaken).into(),
};
CardStatsTemplate {
Ok(CardStatsTemplate {
stats,
revlog,
revlog_titles,
}
.render()
.unwrap()
.unwrap())
}
}

2
rslib/src/stats/graphs.rs
View file

@ -22,7 +22,7 @@ impl Collection {
0
});
let offset = self.local_offset();
let offset = self.local_utc_offset_for_user()?;
let local_offset_secs = offset.local_minus_utc() as i64;
let cards = self.storage.all_searched_cards()?;

2
rslib/src/sync/http.rs
View file

@ -60,8 +60,6 @@ pub struct MetaIn {
pub struct StartIn {
#[serde(rename = "minUsn")]
pub client_usn: Usn,
#[serde(rename = "offset", default)]
pub minutes_west: Option<i32>,
#[serde(rename = "lnewer")]
pub local_is_newer: bool,
}

12
rslib/src/sync/http_client.rs
View file

@ -73,15 +73,9 @@ impl SyncServer for HTTPSyncClient {
self.json_request(&input).await
}
async fn start(
&mut self,
client_usn: Usn,
minutes_west: Option<i32>,
local_is_newer: bool,
) -> Result<Graves> {
async fn start(&mut self, client_usn: Usn, local_is_newer: bool) -> Result<Graves> {
let input = SyncRequest::Start(StartIn {
client_usn,
minutes_west,
local_is_newer,
});
self.json_request(&input).await
@ -370,13 +364,13 @@ mod test {
})
));
let _graves = syncer.start(Usn(1), None, true).await?;
let _graves = syncer.start(Usn(1), true).await?;
// aborting should now work
syncer.abort().await?;
// start again, and continue
let _graves = syncer.start(Usn(1), None, true).await?;
let _graves = syncer.start(Usn(1), true).await?;
syncer.apply_graves(Graves::default()).await?;

6
rslib/src/sync/mod.rs
View file

@ -404,11 +404,7 @@ where
async fn start_and_process_deletions(&mut self, state: &SyncState) -> Result<()> {
let remote: Graves = self
.remote
.start(
state.usn_at_last_sync,
self.col.get_local_mins_west(),
state.local_is_newer,
)
.start(state.usn_at_last_sync, state.local_is_newer)
.await?;
debug!(self.col.log, "removed on remote";

17
rslib/src/sync/server.rs
View file

@ -15,12 +15,7 @@ use super::ChunkableIDs;
#[async_trait(?Send)]
pub trait SyncServer {
async fn meta(&self) -> Result<SyncMeta>;
async fn start(
&mut self,
client_usn: Usn,
minutes_west: Option<i32>,
local_is_newer: bool,
) -> Result<Graves>;
async fn start(&mut self, client_usn: Usn, local_is_newer: bool) -> Result<Graves>;
async fn apply_graves(&mut self, client_chunk: Graves) -> Result<()>;
async fn apply_changes(&mut self, client_changes: UnchunkedChanges)
-> Result<UnchunkedChanges>;
@ -80,20 +75,12 @@ impl SyncServer for LocalServer {
})
}
async fn start(
&mut self,
client_usn: Usn,
minutes_west: Option<i32>,
client_is_newer: bool,
) -> Result<Graves> {
async fn start(&mut self, client_usn: Usn, client_is_newer: bool) -> Result<Graves> {
self.server_usn = self.col.usn()?;
self.client_usn = client_usn;
self.client_is_newer = client_is_newer;
self.col.storage.begin_rust_trx()?;
if let Some(mins) = minutes_west {
self.col.set_local_mins_west(mins)?;
}
self.col.storage.pending_graves(client_usn)
}

8
rslib/src/timestamp.rs
View file

@ -27,6 +27,14 @@ impl TimestampSecs {
pub(crate) fn date_string(self, offset: FixedOffset) -> String {
offset.timestamp(self.0, 0).format("%Y-%m-%d").to_string()
}
pub fn local_utc_offset(self) -> FixedOffset {
*Local.timestamp(self.0, 0).offset()
}
pub fn datetime(self, utc_offset: FixedOffset) -> DateTime<FixedOffset> {
utc_offset.timestamp(self.0, 0)
}
}
impl TimestampMillis {