diff options
| -rw-r--r-- | src/calendarstore.cpp | 26 | ||||
| -rw-r--r-- | src/calendarsync.cpp | 1 | ||||
| -rw-r--r-- | src/calendarwindow.cpp | 5 | ||||
| -rw-r--r-- | src/calendarwriter.cpp | 5 | ||||
| -rw-r--r-- | src/config.cpp | 3 | ||||
| -rw-r--r-- | tests/test_calendarstore.cpp | 49 | ||||
| -rw-r--r-- | tests/test_calendarwindow.cpp | 53 | ||||
| -rw-r--r-- | tests/test_config.cpp | 15 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 3 | ||||
| -rw-r--r-- | translations/qtmaildir_it_IT.ts | 4 |
10 files changed, 162 insertions, 2 deletions
diff --git a/src/calendarstore.cpp b/src/calendarstore.cpp index ef67eba..619ffb9 100644 --- a/src/calendarstore.cpp +++ b/src/calendarstore.cpp @@ -363,8 +363,25 @@ void writeFields(icalcomponent *root, icalcomponent *c, const EventEdit &edit, b if (!master || edit.repeat.custom) return; // a custom RRULE is never rewritten (spec, repeat control) removeAll(c, ICAL_RRULE_PROPERTY); - if (edit.repeat.freq == RepeatRule::Freq::None) + if (edit.repeat.freq == RepeatRule::Freq::None) { + // Stopping a series orphans its RECURRENCE-ID overrides: occurrences() + // skips the override loop for a non-repeating event, so they would + // vanish from every view while staying in the synced file, ready to + // spring back if a repeat is re-enabled. EXDATEs are inert once the + // series does not repeat, and removing them would drop data, so they + // stay. Collect first: removing a component mid-walk invalidates it. + QList<icalcomponent *> doomed; + for (icalcomponent *v = icalcomponent_get_first_component(root, ICAL_VEVENT_COMPONENT); + v; v = icalcomponent_get_next_component(root, ICAL_VEVENT_COMPONENT)) { + if (v != c && icalcomponent_get_first_property(v, ICAL_RECURRENCEID_PROPERTY)) + doomed.append(v); + } + for (icalcomponent *v : doomed) { + icalcomponent_remove_component(root, v); + icalcomponent_free(v); + } return; + } // UNTIL follows DTSTART's type: a DATE for all-day, else UTC (RFC 5545). QString until; if (edit.repeat.end == RepeatRule::End::Until) { @@ -667,8 +684,13 @@ bool sameMeaning(const QByteArray &a, const QByteArray &b) || p.end != q.end || p.summary != q.summary || p.cancelled != q.cancelled) return false; } + // EXDATE properties are a SET: a server may reorder them, so compare + // sorted copies rather than the lists as read. + QList<QDateTime> xd = x.exdates, yd = y.exdates; + std::sort(xd.begin(), xd.end()); + std::sort(yd.begin(), yd.end()); return x.summary == y.summary && x.start == y.start && x.end == y.end - && x.allDay == y.allDay && x.repeat == y.repeat && x.exdates == y.exdates; + && x.allDay == y.allDay && x.repeat == y.repeat && xd == yd; } } // namespace CalendarStore diff --git a/src/calendarsync.cpp b/src/calendarsync.cpp index 38d0c07..fa85c1c 100644 --- a/src/calendarsync.cpp +++ b/src/calendarsync.cpp @@ -29,6 +29,7 @@ CalendarSync::CalendarSync(const QString &command, int delayMs, QObject *parent) [this]() { m_output += m_process.readAll(); }); connect(&m_process, &QProcess::finished, this, [this](int code, QProcess::ExitStatus status) { + m_output += m_process.readAll(); // drain the last chunk, delivered with finished emit finished(status == QProcess::NormalExit && code == 0, QString::fromLocal8Bit(m_output)); if (m_again) { diff --git a/src/calendarwindow.cpp b/src/calendarwindow.cpp index 7079c62..d9c1da2 100644 --- a/src/calendarwindow.cpp +++ b/src/calendarwindow.cpp @@ -145,6 +145,8 @@ CalendarWindow::CalendarWindow(const Config &config, const QStringList &ownAddre if (index >= 0) m_collectionBox->setCurrentIndex(index); (agenda ? m_agendaButton : m_monthButton)->click(); + if (agenda) + m_agendaView->scrollToDay(QDate::currentDate()); } CalendarWindow::~CalendarWindow() = default; @@ -679,6 +681,9 @@ bool CalendarWindow::applyChanges(const QList<Change> &changes, bool reverse) for (int j = done.size() - 1; j >= 0; --j) { QString ignored; CalendarWriter::replace(done[j].path, done[j].after, done[j].before, &ignored); + // The write was reverted, so it must not be reported as a + // difference at the next sync. + m_written.remove(done[j].path); } status(r == CalendarWriter::Result::Stale ? tr("The event changed on disk, probably from a sync. Check it and try again.") diff --git a/src/calendarwriter.cpp b/src/calendarwriter.cpp index 4c81a37..b39dced 100644 --- a/src/calendarwriter.cpp +++ b/src/calendarwriter.cpp @@ -49,6 +49,11 @@ Result replace(const QString &path, const std::optional<QByteArray> &expected, return Result::Ok; } + // ponytail: the stale read above and QSaveFile::commit() are not one + // atomic operation; a cron sync landing between them is lost. The writer + // holds no lock, because the sync command owns the flock. Upgrade path: + // take the same lock here if that window ever costs real data. + // // QSaveFile is the platform's atomic write: a temporary in the same // directory, renamed on commit(). Its temporary is named "<name>.XXXXXX", // which does not end in .ics, so vdirsyncer never lists it. diff --git a/src/config.cpp b/src/config.cpp index 23c08f7..14d7775 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -303,6 +303,9 @@ void Config::load(const QString &path) const int value = calDelay.toString().toInt(&ok); if (ok && value >= 0) m_calendarSyncDelayMs = value; + else if (ok) + addProblem(tr("Calendar sync delay %1 is out of range; using the default.") + .arg(calDelay.toString())); else addProblem(tr("Calendar sync delay '%1' is not a number; using the default.") .arg(calDelay.toString())); diff --git a/tests/test_calendarstore.cpp b/tests/test_calendarstore.cpp index 524304e..8aa526e 100644 --- a/tests/test_calendarstore.cpp +++ b/tests/test_calendarstore.cpp @@ -152,10 +152,12 @@ private slots: void aNewEventIsCompleteAndEmbedsItsZone(); void sameMeaningIgnoresFormatting(); void sameMeaningMatchesOverridesRegardlessOfOrder(); + void sameMeaningTreatsExdatesAsASet(); void sameMeaningRejectsDifferentUids(); void editingOneOccurrenceWritesAnOverride(); void editingTheSameOccurrenceAgainReplacesItsOverride(); + void stoppingASeriesDropsItsOverrides(); void deletingOneOccurrenceAddsAnExdateAndDropsItsOverride(); void deletingOneOccurrenceKeepsExistingExdates(); @@ -595,6 +597,25 @@ void TestCalendarStore::sameMeaningMatchesOverridesRegardlessOfOrder() QVERIFY(!CalendarStore::sameMeaning(a, ics(master + second + changed))); } +void TestCalendarStore::sameMeaningTreatsExdatesAsASet() +{ + // EXDATE properties are a set; a server may reorder them on the round + // trip, and a list comparison would then warn about a version it kept. + const QByteArray a = ics(vevent(QStringLiteral( + "UID:ex@example.org\r\nDTSTART:20260922T080000Z\r\n" + "EXDATE:20260923T080000Z\r\nEXDATE:20260924T080000Z\r\n"))); + const QByteArray b = ics(vevent(QStringLiteral( + "UID:ex@example.org\r\nDTSTART:20260922T080000Z\r\n" + "EXDATE:20260924T080000Z\r\nEXDATE:20260923T080000Z\r\n"))); + QVERIFY(CalendarStore::sameMeaning(a, b)); + + // Only the order is ignored, not the contents. + const QByteArray changed = ics(vevent(QStringLiteral( + "UID:ex@example.org\r\nDTSTART:20260922T080000Z\r\n" + "EXDATE:20260923T080000Z\r\nEXDATE:20260925T080000Z\r\n"))); + QVERIFY(!CalendarStore::sameMeaning(a, changed)); +} + void TestCalendarStore::sameMeaningRejectsDifferentUids() { // Identical fields, different events: the UID is the identity. @@ -647,6 +668,34 @@ void TestCalendarStore::editingTheSameOccurrenceAgainReplacesItsOverride() QCOMPARE(e.overrides[0].start, rome(22, 17)); } +void TestCalendarStore::stoppingASeriesDropsItsOverrides() +{ + // A series with one overridden occurrence, then the repeat is stopped + // (Scope::All with RepeatRule() = Freq::None). The override is not a real + // occurrence any more: occurrences() never reads it for a non-repeating + // event, so leaving it in the file is invalid iCalendar that other clients + // may render as a phantom, and it would spring back if a repeat were + // re-enabled. EXDATEs are left inert. + const CalEvent series = parse(vevent(kDailySeries)); + EventEdit edit = editOf(series); + edit.summary = QStringLiteral("Just this one"); + edit.start = rome(22, 15); + edit.end = rome(22, 16); + const QByteArray withOverride = CalendarStore::applyEdit( + series.rawText, edit, CalendarStore::Scope::ThisOccurrence, rome(22, 10)); + QCOMPARE(reparse(withOverride).overrides.size(), 1); + + EventEdit stop = editOf(reparse(withOverride)); + stop.repeat = RepeatRule(); // does not repeat + const QByteArray after = CalendarStore::applyEdit( + withOverride, stop, CalendarStore::Scope::All, {}); + + const CalEvent e = reparse(after); + QVERIFY(e.overrides.isEmpty()); + QVERIFY2(!after.contains("RECURRENCE-ID"), after.constData()); + QVERIFY2(!after.contains("RRULE"), after.constData()); +} + void TestCalendarStore::deletingOneOccurrenceAddsAnExdateAndDropsItsOverride() { const CalEvent series = parse(vevent(kDailySeries)); diff --git a/tests/test_calendarwindow.cpp b/tests/test_calendarwindow.cpp index c52f245..5591b2b 100644 --- a/tests/test_calendarwindow.cpp +++ b/tests/test_calendarwindow.cpp @@ -27,6 +27,7 @@ #include <QMessageBox> #include <QPushButton> #include <QSpinBox> +#include <QStatusBar> #include <QTemporaryDir> #include <QTimer> #include <QToolBar> @@ -126,6 +127,7 @@ private slots: void theToolbarCarriesTheNewEventAction(); void aFreshWindowShowsTheCurrentMonth(); void aStaleSaveCanBeRetried(); + void aRolledBackWriteIsNotReportedLostAfterSync(); }; void TestCalendarWindow::loadsAndSelects() @@ -267,5 +269,56 @@ void TestCalendarWindow::aStaleSaveCanBeRetried() QVERIFY(!w->isEditing()); } +void TestCalendarWindow::aRolledBackWriteIsNotReportedLostAfterSync() +{ + // applyChanges records every successful write in m_written for the next + // sync to verify. A later change in the same call fails, rolling the + // earlier one back, so that write never reached disk and must be + // forgotten. Keeping it makes the next sync compare the file against + // bytes that were reverted and warn that the server kept a different + // version, a difference that never happened. + QTemporaryDir dir; + const QString cal = dir.filePath(QStringLiteral("cal")); + const QString one = cal + QStringLiteral("/a/one.ics"); + const QString two = cal + QStringLiteral("/a/two.ics"); + const QByteArray original = + eventText(QStringLiteral("one@example.org"), QStringLiteral("One"), 22); + writeFile(one, original); + writeFile(two, eventText(QStringLiteral("two@example.org"), QStringLiteral("Two"), 23)); + + // A real sync command, so the post-sync check actually runs. + const QString ini = dir.filePath(QStringLiteral("qtmaildir.conf")); + writeFile(ini, QStringLiteral("[general]\ncalendars_dir = %1\n" + "calendar_sync_command = /bin/true\n" + "calendar_sync_delay_ms = 0\n").arg(cal).toUtf8()); + Config config; + config.load(ini); + std::unique_ptr<CalendarWindow> w( + new CalendarWindow(config, { QStringLiteral("me@example.org") }, + dir.filePath(QStringLiteral("uistate.conf")))); + + // Change 1 succeeds; change 2 is stale (its expected bytes do not match a + // missing file), so change 1 is rolled back. + const QByteArray rolled = + eventText(QStringLiteral("one@example.org"), QStringLiteral("Rolled"), 22); + const QList<CalendarWindow::Change> changes = { + { one, original, rolled }, + { cal + QStringLiteral("/a/ghost.ics"), QByteArray("gone"), QByteArray("x") }, + }; + QVERIFY(!w->applyChanges(changes, false)); + QCOMPARE(read(one), original); // the first change was reverted + + // A later successful write triggers the sync. The rolled-back path must + // not reappear in the post-sync check: with the fix the sync reports + // clean, without it the stale entry produces a false "different version". + const QByteArray twoAfter = + eventText(QStringLiteral("two@example.org"), QStringLiteral("Two, edited"), 23); + QVERIFY(w->applyChanges({ { two, read(two), twoAfter } }, false)); + + QTRY_VERIFY_WITH_TIMEOUT( + w->statusBar()->currentMessage().contains(QStringLiteral("Calendars synced.")), + 5000); +} + QTEST_MAIN(TestCalendarWindow) #include "test_calendarwindow.moc" diff --git a/tests/test_config.cpp b/tests/test_config.cpp index be343d1..b0597de 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -154,6 +154,7 @@ private slots: void calendarSyncCommandSurvivesACommaAndDefaults(); void calendarsDirThatDoesNotExistIsReported(); void calendarSyncDelayRejectsGarbage(); + void aNegativeCalendarSyncDelayIsOutOfRange(); }; static QString writeIni(const QTemporaryDir &dir, const QString &body) @@ -2963,5 +2964,19 @@ void TestConfig::calendarSyncDelayRejectsGarbage() QVERIFY(!config.problems().isEmpty()); } +void TestConfig::aNegativeCalendarSyncDelayIsOutOfRange() +{ + // "-5" parses, so "is not a number" would be a lie; the value is simply + // outside what the key accepts. It falls back to the default and says so. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n" + "calendar_sync_delay_ms=-5\n"))); + QCOMPARE(config.calendarSyncDelayMs(), 2000); + const QString joined = config.problems().join(QLatin1Char('\n')); + QVERIFY2(joined.contains(QStringLiteral("out of range")), qPrintable(joined)); + QVERIFY2(!joined.contains(QStringLiteral("not a number")), qPrintable(joined)); +} + QTEST_MAIN(TestConfig) #include "test_config.moc" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 5446780..f6f75aa 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -8833,6 +8833,9 @@ void TestMainWindow::theCalendarActionWithNoCalendarOpensNothing() MainWindow window(config); auto *action = window.findChild<QAction *>(QStringLiteral("calendar")); QVERIFY2(action, "no action named calendar"); + // Enabled, not disabled: a future edit that greys it out would otherwise + // make the trigger below a no-op and leave this test passing vacuously. + QVERIFY2(action->isEnabled(), "the calendar action is disabled"); action->trigger(); diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index 6012a90..35702ae 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -469,6 +469,10 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>Il ritardo di sincronizzazione del calendario '%1' non è un numero; uso il valore predefinito.</translation> </message> <message> + <source>Calendar sync delay %1 is out of range; using the default.</source> + <translation>Il ritardo di sincronizzazione del calendario %1 è fuori intervallo; uso il valore predefinito.</translation> + </message> + <message> <source>Language '%1' is not a locale name; using the system language. Expected something like 'it' or 'it_IT'.</source> <translation>'%1' non è un nome di locale; verrà usata la lingua di sistema. Atteso qualcosa come 'it' o 'it_IT'.</translation> </message> |