diff --git a/test/UnitWOPISaveOnExit.cpp b/test/UnitWOPISaveOnExit.cpp index 7dfe5274a..ee8d28d78 100644 --- a/test/UnitWOPISaveOnExit.cpp +++ b/test/UnitWOPISaveOnExit.cpp @@ -308,17 +308,19 @@ public: /// Test upload behavior with always_save_on_exit. /// The test verifies that an unmodified document -/// is *not* uploaded when always_save_on_exit=true. -class UnitSaveOnExitUnmodified : public WopiTestServer +/// is *not* uploaded when always_save_on_exit=true +/// and is closed (ownertermination). +class UnitSaveOnExitUnmodifiedClosed : public WopiTestServer { using Base = WopiTestServer; +protected: STATE_ENUM(Phase, Load, WaitLoadStatus, WaitDestroy, Done) _phase; public: - UnitSaveOnExitUnmodified() - : Base("UnitSaveOnExitUnmodified") + UnitSaveOnExitUnmodifiedClosed(const std::string& name = "UnitSaveOnExitUnmodifiedClosed") + : Base(name) , _phase(Phase::Load) { } @@ -340,13 +342,16 @@ public: return nullptr; } - /// The document is loaded. - bool onDocumentLoaded(const std::string& message) override + /// Wait for ModifiedStatus=false before closing. + /// This is sent right after loading. + bool onDocumentUnmodified(const std::string& message) override { LOG_TST("Got: [" << message << ']'); LOK_ASSERT_STATE(_phase, Phase::WaitLoadStatus); TRANSITION_STATE(_phase, Phase::WaitDestroy); + + LOG_TST("Closing document"); WSD_CMD("closedocument"); return true; @@ -395,10 +400,41 @@ public: } }; +/// Test upload behavior with always_save_on_exit. +/// The test verifies that an unmodified document +/// is *not* uploaded when always_save_on_exit=true +/// and is disconnected. +class UnitSaveOnExitUnmodifiedDisconnect : public UnitSaveOnExitUnmodifiedClosed +{ +public: + UnitSaveOnExitUnmodifiedDisconnect() + : UnitSaveOnExitUnmodifiedClosed("UnitSaveOnExitUnmodifiedDisconnect") + { + } + + /// Wait for ModifiedStatus=false before disconnecting. + /// This is sent right after loading. + bool onDocumentUnmodified(const std::string& message) override + { + LOG_TST("Got: [" << message << ']'); + LOK_ASSERT_STATE(_phase, Phase::WaitLoadStatus); + + TRANSITION_STATE(_phase, Phase::WaitDestroy); + + // Disconnect to trigger the auto-save logic. + LOG_TST("Disconnecting"); + deleteSocketAt(0); + + return true; + } +}; + UnitBase** unit_create_wsd_multi(void) { - return new UnitBase* [4] { - new UnitWOPISaveOnExit(), new UnitSaveOnExitSaved(), new UnitSaveOnExitUnmodified(), nullptr + return new UnitBase* [5] + { + new UnitWOPISaveOnExit(), new UnitSaveOnExitSaved(), new UnitSaveOnExitUnmodifiedClosed(), + new UnitSaveOnExitUnmodifiedDisconnect(), nullptr }; } diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index ce88e17e9..259edeb2b 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -2725,7 +2725,12 @@ std::size_t DocumentBroker::removeSession(const std::shared_ptr& const std::size_t activeSessionCount = countActiveSessions(); const bool lastEditableSession = session->isEditable() && !haveAnotherEditableSession(id); - const bool dontSaveIfUnmodified = !_alwaysSaveOnExit; + // Forcing a save when always_save_on_exit=true creates a new + // file on disk, with a new timestamp, which makes it hard to + // avoid uploading when there really isn't any modifications. + // Instead, we rely on always issuing a save through forced + // auto-save and expect Core has the correct modified flag. + constexpr bool dontSaveIfUnmodified = true; LOG_INF("Removing session [" << id << "] on docKey [" << _docKey << "]. Have " << _sessions.size() << " sessions (" << activeSessionCount