When the document isn't modified and we save,
we get back "unmodified" failure from Core.
This has the unwanted side-effect of skipping
uploading.
As we now decide whether to upload or not
based on the file timestamp on disk,
this logic has no purpose and in fact can
cause a lot of grief.
Removing this outdated logic has the nice
side-effect of simplifying the code, as
the save result doesn't need to be propagated.
Also, save failures are now communicated to
clients at the point of handling the save
response, not when uploading (which is
a distinct stage). And upload failures
are reported separately.
Change-Id: Ia465a0069280fb6bea47e413f0d885565f0bbb3d
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
And don't load documents when the ShutdownRequestedFlag
is set, not just the TerminationFlag.
Change-Id: I94c720d6fd7c361549bc905e093886619e8f188b
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
It is possible to lose the last writable session
during unloading and get stuck forever.
This can happen in the following scenario (as observed
in a rare test run):
1. always_save_on_exit is true.
2. The last/only writable session disconnects.
3. We save and upload.
4. We attempt to stop because the clients have disconnected,
but because we are uploading, it doesn't flag to stop.
5. Uploading fails repeatedly until we reach the limit.
6. We endlessly check to save and upload and even though
there is nothing to save, we couldn't even check for
the need to upload, getting stuck.
With this patch the above scenario is not possible.
Change-Id: I19e7b621f2b452a9f18964f5f19d6eb378a48797
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This adds dumpState to DocumentState, SaveManager,
and StorageManager classes, and dumps all the
missing members.
Also, normalize the format and make it symmetric
and consistent.
Change-Id: Ie0cc8e07d13de60c33d64cd621abf4e815a4ef94
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
When the last client connection is closed,
there is UI (or user) to provide input
on the document conflict dialog. In this
case, we detect that the situation is final
and unresolvable and we simply give up.
We log a warning and dump the document state
before terminating.
Change-Id: I111a446b8743a0d16b7ed8b39751a419036c927f
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
We no store the origina modified time of the document
as we receive from the storage server in string
form and send it back as-is. This avoids any
potential issues with the roundtrip of conversion
to and from a timestamp.
Change-Id: I524bea8f36c3ce62dcd00c4fe6a1e7e083287ed1
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Probably the last remaining incorrectly labelled
helper that checks for in-progress uploads, but
had retained the old and misleading label of 'save'.
Change-Id: I693275b1559f3dae4e9e3ab2408d997f56ff86e3
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
PDFs are view_comment type documents where
editing is not supported, but adding comments
are. This means that we do get ModifiedStatus=true
but we never get ModifiedStatus=false after
.uno:Save. This is because the save command is
handled in a special way in Core by invoking
save-as instead, which doesn't reset the
ModifiedStatus.
The issue with this was that DocBroker was
stuck on trying to save the document
before unloading, thinking it was modified
due to the stuck ModifiedStatus. Here,
we change the definition of isPossiblyModified()
to ignore the ModifiedStatus for such documents.
Instead, we rely on the last save being successful
and that no new user input exists past the last
save request.
In addition, we now have a new Cypress test
that reproduced the failure without the fix
and now passes with the fix.
Change-Id: Ida9d486ac93a588b9007c5e4583d8bf3c090a62d
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This fixes failures in UnitCopyPaste and
TileCacheTests::testWireIDFilteringOnWSDSide where
the documents are closed while isModified() is true.
Change-Id: I6de683530df9b7987ad87897e9ce70b5fc3d3a15
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
(cherry picked from commit 65105263fbed89d787ac59ca77f64896bacf6832)
We make sure that we always check for last-minute
modifications before stopping DocBroker. This covers
the cases where there are modifications that we need
to save and/or saved data that needs uploading.
This save, upload, and stop logic is now in a helper
that cleanly handles this exit-time check.
Change-Id: Ibee0e7769a396b205de955f26700ebcb27d5ac95
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
(cherry picked from commit 69f9d86c8b8a477b5a0cf1910eb1b7498eaceeac)
RequestDetails manages URLs and their components.
The DocKey is based on the URL, so it makes sense
to be provided by RequestDetails.
Change-Id: If99e677a2e5a227f09b5035f388c4da587084db9
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
And cleanup around DocKey and URI handling.
Change-Id: Ieef83197ac9b04e22af92eee217e336bc2b31761
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Unloading duration depends on the time to save
and upload the document, during which a new view
might be opened. We now allow aborting the unloading
and resume reusing the document as-is.
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
(cherry picked from commit 8f299eb2cdfe0ef869a8c90fbc602a93ba2fcb73)
Change-Id: I7bc485177929c0d1c9ef12b02672db493ac402c9
This adds a new service render-search-result, which renders an
image of the position where, the search fund a hit. The search
result contains the information in which object or paragraph,
which is just the node id and node type from the document model.
The service takes the document and the search result (xml file)
as the input and returns a PNG image as the output.
Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>
Change-Id: Iffc158c5b0a3c9a63f8d05830314c9bc1616b3b1
Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>
When async save was started but we want to stop DocumentBroker
- we shouldn't kill the socket which listens to save result.
Because when we kill socket we will destroy DocumentBroker
before saving is completed, this allows for a minimal time
to connect again to the document and receive old content
from the WOPI storage. When we wait for async save result
we will keep DocumentBroker alive and next session can be
connected and receive correct content.
fixes: https://github.com/CollaboraOnline/online/issues/2747
Signed-off-by: Szymon Kłos <szymon.klos@collabora.com>
Change-Id: I5505a5ccc1f3d6928c723bddfea16bf5c5798d1e
We ask for save as for the plain/text type documents
such as csv or txt. Notify other views about it so
they can follow one to the new document
Signed-off-by: merttumer <mert.tumer@collabora.com>
Change-Id: I4cdcec6148d1064ad4e90ad9be2ce483c19a8eda
Templates are special because the file that gets
loaded is not in the same format as the final one.
An implicit save-as is issued in Kit for template
loads right after loading to create the final
document format. It is this file (as opposed to the
template) that needs to be uploaded, which we do
right after we get the 'status:' message in
DocBroker to actually create the first version
in storage.
Change-Id: I23cbf527f1059b0b4059f15069737ab35ac860e7
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The client always issued a save before renameFile.
This worked fine until we make uploading asynchronous.
Now that uploading is done asychronously, the renameFile
command arrives while uploading is still in progress
and fails because renaming is done via the same
storage API used for uploading.
It is best to do both the saving (including uploading)
and renaming directly in WSD, instead of relying on
the client to save explicitly before renaming.
Change-Id: I99f6294f4c83130bc13ec8e3b013ba0192f0f84c
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Some user-initiated activities, such as renaming
the document filename, is either a multi-stage
activity or involves async operations.
Using a new Activity enum in DocumentState, we
now track each of the different activities that
needs special attention or handling.
Change-Id: I9cd2d04b10a97387ce02f5c3cc98b41a1732af6a
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
With async uploading we have to process the
stop-condition only after we get the response from
the async operation. For now, we don't stop
if we fail to upload, giving a chance to retrying.
Timeout should be used in the poll loop to
limit how long we retry.
Change-Id: I8e8f8618f92f34ef2f1853e499f4f8d6ce932b40
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This is to also include the 'force' case that is
needed when unloading a document.
Now that the force flag is reliably set, we don't
need to override it in needToUploadToStorage.
Change-Id: Ic0672e6c7bcb53a17ec6a0cbd2d2f8b434fbc300
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Add the necessary support for handling async
document upload to storage in DocumentBroker.
Change-Id: Id7a58c245d729cef58669bafb60db450f7ebbba2
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Makes it accessible to more code.
Change-Id: I73dd1895defe47ae40873b6155203768055206ec
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Add a new state before load the document,
when Macro Security dialog popup, and avoid
to send the event load timeout.
Change-Id: I5973c5205e90e5447e5478cbab895709a68606f6
Signed-off-by: Henry Castro <hcastro@collabora.com>
Consistent with names elsewhere and less confusing.
Change-Id: Ia72cdef0f6d6cd9589ce54028472e10008cf882c
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
We use download/upload in the storage
and load/save elsewhere, to avoid confusion.
This renames 'save' to 'upload' in the storage
for consistency.
Change-Id: I9ac991c2b52e2586b97c58db02110ff04bfd17d3
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Instead of inheritance we use composition
to preserve the uniqueness of the members
of SaveManager and StorageManager.
Change-Id: Ifad82787e54089d49226085e009ade67bfc7938a
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This will be used to detect whether we need
to upload the document or not.
Change-Id: I2c5f6d058b1a8e0a6ab20c9561b6701413fb5878
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This is to encapsulate and track the
common logic between SaveManager and
StorageManager.
Change-Id: I0c5a59edb8a26b258ba66d65983e2f76198ecbc9
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This is to encapsulate and track the logic
for uploading documents from the Storage.
Change-Id: I5b972151fe9548526755d18063d37cc95949e68f
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
We should only update the save timestamps when we
really save a new version of the document.
Requesting renaming or moving ("save as") in storage
should not be confused with actually saving in Core.
Here we take the first step in this separation,
and we only update the last save response time
when we truely get a response from Core, and
not when we execute a document path/name in storage.
Change-Id: I931c62b306dd3b4906a9e910f844a35fb3f4b6f0
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The SaveManager is responsible for the file on disk.
Change-Id: I92f7843375fdc875bc7d1af3fba387f67a4f0ca5
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The SaveManager is responsible for the document's
save state. It also encapsulates the members
necessary to track the save state and related
data.
Change-Id: I3ed0f1d93f25988b1ad8b1a121a2080288866a53
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This details the design of the DocumentBroker
states and state-machines as they are to be
eventually implemented. Not everything described
is current, but the goal is to document what
the design ought to be, not what it is.
Change-Id: I938177812777af058b46c41a396407d0a083cbc4
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Only the last argument (force) was implicit, and only
in one call (of total two calls). Explicit is better.
Change-Id: Ic26f4dd265f48156d1730f1b95bb70145ca47873
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The loaded flag is used to track whether or not
the document was ever loaded. It does *not* reflect
whether or not the document is currently loaded,
rather it is used to save the document, if necessary,
before unloaded.
This is why we need to track it as a separate boolean,
rather than implicitly tracking this state via the
DocumentState enum, which would not be able to
differentiate between a failed-to-load state and
one of a normal end-of-life, after loading.
Change-Id: I1fc3fb09c31cadf7ebb6b0123e462e78dd0af356
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The markToDestroy flag tracks whether or not the
document is in the process of being unloaded.
Change-Id: I8f2339f75ed6d7c5d318eb1d467d6a9cbc1d61c2
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
DocumentState is responsible for tracking
the state of the document, properly documenting
the different stages of the document lifetime
and encapsulating all the relevant members.
DocumentState is still unused in this commit.
Change-Id: Ic2c8de3a9f2d42c5550c5f4ad5f889040f697890
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
To differentiate between the different stages
of document lifecycle, fetching documents
from the storage is now called 'download',
as opposed to the previous 'load', which is
reserved to loading document in Core.
This is a cosmetic renaming to avoid
confusing usage and intent going forward.
Change-Id: Ib9451e6f73bab19b877a3e6c8fb5b17ba82a06ab
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Saving documents to storage also logs and broadcasts
the result to the users. Return values from these
functions are ignored, and anyway not actionable.
Change-Id: Iaf0dab9c6ac8c593e4df292c71fcb30e8b6d7eeb
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
No advantage in using int when chrono handles
conversion and comparisons transparently for us.
Change-Id: Idc942e7a2557ef979d876f378cf6bb84d3e657cd
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Now chosen log level is propagated to forkit and kits.
Also, admin console users can filter logs according to their channel names on client side.
Change-Id: Ife15a6148ed87533b81e9d63da252c633e74e559
Signed-off-by: Gökay Şatır <gokaysatir@collabora.com>
A number of call-sites, eg. clipboard, or admin-ws were
writing to sockets assuming they could return all the data
in a single series of writes, without needing to poll. As
such they failed to addSocketToPoll on the new poll - eg.
the docBroker. Unfortunately this meant that on EAGAIN
writes, the socket would be closed and the last parts
of a message lost.
Browsers would give net::ERR_CONTENT_LENGTH_MISMATCH 200 (OK)
The situation is/was intermittent, so painful to debug.
On under-loaded developer machines, socket buffers are larger,
so this was seldom seen.
The re-factor forces a transfer to another SocketPoll via
the disposition, except for a couple of corner cases.
Change-Id: I2f1b2f99f179c4fda84464c9241fe434fa527725
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Using "load" and "save" in the storage was a poor
choice of verbs, in hindsight, because these very
same verbs are also used to describe the loading
and saving of documents in Core.
It is more appropriate to label the storage
operations as download and upload, respectively,
to avoid any confusion. This is especially useful
because when reporting we have for some time now
been reporting the results of each of these
stages separately, there is no longer reason
to label them the same.
We already used "upload" and "download" in
some of the logs, but not all.
Change-Id: I0fac9130032e2c3c6dfb4d671c31130265091f0d
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This is in preparation for asynchronous uploading.
Change-Id: I5c9977107b415efd24cbd99c29599b86cfe32933
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The current session is needed only while loading, as it's not
yet in the sessions container. But while saving, broadcasting
to all sessions includes the current session as well, and
we avoid sending duplicate message to the current session.
We also make the broadcast helper a member of DocumentBroker
which simplifies it.
Change-Id: I3bb37cc808d97ba2b772b88474a8c10f7fdff6b7
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
We now download the convert-to files into the
child-root/tmp directory and then move it into
the jail that will convert it. This way ownership
and cleanup become contained within our child-root
and jail subsystems. This reduces the chances of
leaking convert-to files and simplifies the design.
In addition, we avoid an extra file copy and improve
the security of the convert-to API.
Change-Id: I450c24d0d0dc0da447c8072b0701c3b48d07c81b
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
size_t in C and in C++ are not necessarily the same
type. The C++ size_t is in the std namespace. Since
we do include many C headers, and indeed some C++
runtime headers do define size_t for backwards
compatibility, it's easy to mix and match the two
types.
Also, 'using std::size_t;' isn't a great practice,
so removed.
This is not exhaustive, just some low-hanging cases.
Change-Id: I85a36b6fd1acd204274b1869de9bcb94c8b3cf13
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Use hash to identify download and pass that to the client.
This allows us to reduce parameters for download requests.
DocBroker maps download ids to URL in the file system.
Change-Id: I254d4f0ccaf3cff9f038a817c8162510ae228bc5
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/101992
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Tested-by: Michael Meeks <michael.meeks@collabora.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
This reverts commit e83e36bd9b.
Unit test failure was fixed
Change-Id: I2176368278725c1711df3b23eef95de6526c68d5
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/100859
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt@cib.de>
Otherwise client gets a notification that document is unmodified.
This should not happen, as the document in the storage has not been updated
and so it should be considered as modified until saving to storage succeeds.
Change-Id: I6918f97d96a546ce086f622854f4cbeed48d54ae
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/100162
Tested-by: Jenkins
Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt@cib.de>
Use mobile-edit-button for that is permitted.
Change-Id: I4d4c3f21d574abae033bacc69def96aaf6b51567
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98786
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
* Excised TileCacheDesc to improve performance and simplify code.
* clang-tidy suggestions and auto-rewrite fixes.
* Const-correctness.
* Inlined and improved a couple of trivial functions (that are called
often).
* Reduced some logs from INF to DBG as they are only meaningful to devs.
Change-Id: I1c4eb8c63da49aa061afbf3eb68cae23d4d5e7f3
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98661
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Tested-by: Jenkins
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
665b1629de was not correct, as it reported back
the save result of the internal save (which usually succeeds).
Instead we want to know the save result of the remote storage (WOPI/Webdav).
So report that back instead.
Change-Id: Iaaa42b8c817a19c2c77935a6f81c1951fdf2216c
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/97637
Tested-by: Jenkins
Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt@cib.de>
Seems to not cause any serious regressions in the iOS app or in "make
run", but of course I am not able to run a comprehensive check of all
functionality.
Change-Id: I44a0e8d60bdbc0a885db88475961575c5e95ce88
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/93037
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
Also adds ServiceRoot handling for clipboard.
Change-Id: I7bc6591130fcc7d693e59ab8561fb9e99f4e93d5
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/93578
Tested-by: Michael Meeks <michael.meeks@collabora.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
And also compile out ConvertToBroker in mobile apps, it is not
needed there, otherwise it wouldn't compile due to the added check for
nocaps.
Change-Id: I20fe7e3b702e4a1782640a2d0e71a40b1517beb6
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/93510
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Jan Holesovsky <kendy@collabora.com>
