tdf#99744 SaveAs: Report back to loleaflet that the saveas succeeded.

Change-Id: I670c8b4503c1a4c0a88001a1343f6dec2974e044

11 files changed, 119 insertions, 59 deletions

diff --git a/kit/ChildSession.cpp b/kit/ChildSession.cpp
index 9c5063891..b9549c065 100644
--- a/kit/ChildSession.cpp
+++ b/kit/ChildSession.cpp
@@ -992,11 +992,10 @@ bool ChildSession::saveAs(const char* /*buffer*/, int /*length*/, const std::vec
     Poco::URI::encode(url, "", encodedURL);
     Poco::URI::encode(wopiFilename, "", encodedWopiFilename);
 
-    sendTextFrame("saveas: url=" + encodedURL + " wopifilename=" + encodedWopiFilename);
-    std::string successStr = success ? "true" : "false";
-    sendTextFrame("unocommandresult: {"
-            "\"commandName\":\"saveas\","
-            "\"success\":\"" + successStr + "\"}");
+    if (success)
+        sendTextFrame("saveas: url=" + encodedURL + " filename=" + encodedWopiFilename);
+    else
+        sendTextFrame("error: cmd=storage kind=savefailed");
 
     return true;
 }
diff --git a/loleaflet/src/control/Toolbar.js b/loleaflet/src/control/Toolbar.js
index a2d70d79a..4ec419e35 100644
--- a/loleaflet/src/control/Toolbar.js
+++ b/loleaflet/src/control/Toolbar.js
@@ -93,13 +93,10 @@ L.Map.include({
 		}
 
 		this.showBusy(_('Saving...'), false);
-		// TakeOwnership: we are performing a 'real' save-as, the document
-		// is just getting a new place, ie. it will get the
-		// '.uno:ModifiedStatus' upon completion.
 		this._socket.sendMessage('saveas ' +
 			'url=' + url + ' ' +
 			'format=' + format + ' ' +
-			'options=TakeOwnership,' + options);
+			'options=' + options);
 	},
 
 	applyStyle: function (style, familyName) {
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index d83b6afc0..1481f4af2 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -562,10 +562,9 @@ L.Socket = L.Class.extend({
 			this._map._docLayer._debugRenderCount = command.rendercount;
 		}
 		else if (textMsg.startsWith('saveas:')) {
-			textMsg = (textMsg.substring(7)).trim();
-			// var url = textMsg.substring(0, textMsg.indexOf(' '));
-			// var fileName = textMsg.substring(textMsg.indexOf(' '));
-			/// redirect or not?
+			this._map.hideBusy();
+			// var url = command.url; // WOPI url - if needed at some stage
+			// var name = command.name; TODO dialog that the file was saved as "name"
 		}
 		else if (textMsg.startsWith('statusindicator:')) {
 			//FIXME: We should get statusindicator when saving too, no?
@@ -793,6 +792,9 @@ L.Socket = L.Class.extend({
 			else if (tokens[i].substring(0, 5) === 'char=') {
 				command.char = tokens[i].substring(5);
 			}
+			else if (tokens[i].substring(0, 4) === 'url=') {
+				command.url = tokens[i].substring(4);
+			}
 			else if (tokens[i].substring(0, 7) === 'viewid=') {
 				command.viewid = tokens[i].substring(7);
 			}
diff --git a/loleaflet/src/map/handler/Map.WOPI.js b/loleaflet/src/map/handler/Map.WOPI.js
index 7b2a44d43..dc08693e4 100644
--- a/loleaflet/src/map/handler/Map.WOPI.js
+++ b/loleaflet/src/map/handler/Map.WOPI.js
@@ -221,15 +221,12 @@ L.Map.WOPI = L.Handler.extend({
 			this._postMessage({msgId: 'Get_Export_Formats_Resp', args: exportFormatsResp});
 		}
 		else if (msg.MessageId === 'Action_SaveAs') {
-			/* TODO
 			if (msg.Values) {
-				if (msg.Values.Filename === null || msg.Values.Filename === undefined) {
-					msg.Values.Filename = '';
+				if (msg.Values.Filename !== null && msg.Values.Filename !== undefined) {
+					this._map.showBusy(_('Creating copy...'), false);
+					map.saveAs('wopi:' + msg.Values.Filename);
 				}
-				this.showBusy(_('Creating copy...'), false);
-				map.saveAs(msg.Values.Filename);
 			}
-			*/
 		}
 	},
 
diff --git a/test/UnitWOPISaveAs.cpp b/test/UnitWOPISaveAs.cpp
index 82e89de07..cb9ff9c3e 100644
--- a/test/UnitWOPISaveAs.cpp
+++ b/test/UnitWOPISaveAs.cpp
@@ -31,7 +31,7 @@ public:
     {
     }
 
-    void assertPutFileRelativeRequest(const Poco::Net::HTTPRequest& request) override
+    void assertPutRelativeFileRequest(const Poco::Net::HTTPRequest& request) override
     {
         // spec says UTF-7...
         CPPUNIT_ASSERT_EQUAL(std::string("/jan/hole+AWE-ovsk+AP0-/hello world.txt"), request.get("X-WOPI-SuggestedTarget"));
diff --git a/test/WopiTestServer.hpp b/test/WopiTestServer.hpp
index 6324114d3..4f0e14c6e 100644
--- a/test/WopiTestServer.hpp
+++ b/test/WopiTestServer.hpp
@@ -64,7 +64,7 @@ public:
     {
     }
 
-    virtual void assertPutFileRelativeRequest(const Poco::Net::HTTPRequest& /*request*/)
+    virtual void assertPutRelativeFileRequest(const Poco::Net::HTTPRequest& /*request*/)
     {
     }
 
@@ -140,17 +140,14 @@ protected:
         }
         else if (request.getMethod() == "POST" && (uriReq.getPath() == "/wopi/files/0" || uriReq.getPath() == "/wopi/files/1"))
         {
-            LOG_INF("Fake wopi host request, handling PutFileRelative: " << uriReq.getPath());
+            LOG_INF("Fake wopi host request, handling PutRelativeFile: " << uriReq.getPath());
 
             CPPUNIT_ASSERT_EQUAL(std::string("PUT_RELATIVE"), request.get("X-WOPI-Override"));
 
-            assertPutFileRelativeRequest(request);
+            assertPutRelativeFileRequest(request);
 
-            Poco::URI wopiURL(helpers::getTestServerURI() + "/wopi/files/1");
-            std::string url;
-            Poco::URI::encode(wopiURL.toString(), ":/?", url);
-
-            std::string content = "{ \"Name\":\"hello world.txt\", \"Url\":\"" + url + "\" }";
+            std::string wopiURL = helpers::getTestServerURI() + "/something wopi/files/1?access_token=anything";
+            std::string content = "{ \"Name\":\"hello world.txt\", \"Url\":\"" + wopiURL + "\" }";
 
             std::ostringstream oss;
             oss << "HTTP/1.1 200 OK\r\n"
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 63fa9eb9c..d3cf71067 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -664,7 +664,7 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
         }
 
         std::string encodedWopiFilename;
-        if (!getTokenString(tokens[2], "wopifilename", encodedWopiFilename))
+        if (!getTokenString(tokens[2], "filename", encodedWopiFilename))
         {
             LOG_ERR("Bad syntax for: " << firstLine);
             return false;
@@ -703,11 +703,11 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
             // Normal SaveAs - save to Storage and log result.
             if (resultURL.getScheme() == "file" && !resultURL.getPath().empty())
             {
+                // this also sends the saveas: result
                 docBroker->saveAsToStorage(getId(), resultURL.getPath(), wopiFilename);
             }
-
-            if (!isCloseFrame())
-                forwardToClient(payload);
+            else
+                sendTextFrame("error: cmd=storage kind=savefailed");
         }
         else
         {
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 1bac7182f..77559bfca 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -675,7 +675,7 @@ bool DocumentBroker::saveToStorageInternal(const std::string& sessionId,
 
     assert(_storage && _tileCache);
     StorageBase::SaveResult storageSaveResult = _storage->saveLocalFileToStorage(auth, saveAsPath, saveAsFilename);
-    if (storageSaveResult == StorageBase::SaveResult::OK)
+    if (storageSaveResult.getResult() == StorageBase::SaveResult::OK)
     {
         if (!isSaveAs)
         {
@@ -697,12 +697,21 @@ bool DocumentBroker::saveToStorageInternal(const std::string& sessionId,
         }
         else
         {
-            Log::debug() << "Saved As docKey [" << _docKey << "] to URI [" << uri
-                         << "] successfully.";
+            // normalize the url (mainly to " " -> "%20")
+            std::string url = Poco::URI(storageSaveResult.getSaveAsUrl()).toString();
+
+            // encode the name
+            std::string encodedName;
+            Poco::URI::encode(storageSaveResult.getSaveAsName(), "", encodedName);
+
+            it->second->sendTextFrame("saveas: url=" + url + " filename=" + encodedName);
+
+            Log::debug() << "Saved As docKey [" << _docKey << "] to URI [" << url
+                         << " with name '" << encodedName << "'] successfully.";
         }
         return true;
     }
-    else if (storageSaveResult == StorageBase::SaveResult::DISKFULL)
+    else if (storageSaveResult.getResult() == StorageBase::SaveResult::DISKFULL)
     {
         LOG_WRN("Disk full while saving docKey [" << _docKey << "] to URI [" << uri <<
                 "]. Making all sessions on doc read-only and notifying clients.");
@@ -714,18 +723,18 @@ bool DocumentBroker::saveToStorageInternal(const std::string& sessionId,
             sessionIt.second->sendTextFrame("error: cmd=storage kind=savediskfull");
         }
     }
-    else if (storageSaveResult == StorageBase::SaveResult::UNAUTHORIZED)
+    else if (storageSaveResult.getResult() == StorageBase::SaveResult::UNAUTHORIZED)
     {
         LOG_ERR("Cannot save docKey [" << _docKey << "] to storage URI [" << uri << "]. Invalid or expired access token. Notifying client.");
         it->second->sendTextFrame("error: cmd=storage kind=saveunauthorized");
     }
-    else if (storageSaveResult == StorageBase::SaveResult::FAILED)
+    else if (storageSaveResult.getResult() == StorageBase::SaveResult::FAILED)
     {
         //TODO: Should we notify all clients?
         LOG_ERR("Failed to save docKey [" << _docKey << "] to URI [" << uri << "]. Notifying client.");
         it->second->sendTextFrame("error: cmd=storage kind=savefailed");
     }
-    else if (storageSaveResult == StorageBase::SaveResult::DOC_CHANGED)
+    else if (storageSaveResult.getResult() == StorageBase::SaveResult::DOC_CHANGED)
     {
         LOG_ERR("PutFile says that Document changed in storage");
         _documentChangedInStorage = true;
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index af60c2374..95a489cbb 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -324,7 +324,7 @@ StorageBase::SaveResult LocalStorage::saveLocalFileToStorage(const Authorization
         return StorageBase::SaveResult::FAILED;
     }
 
-    return StorageBase::SaveResult::OK;
+    return StorageBase::SaveResult(StorageBase::SaveResult::OK);
 }
 
 namespace {
@@ -674,7 +674,7 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization&
     LOG_INF("Uploading URI via WOPI [" << uriObject.toString() << "] from [" << _jailedFilePath + "].");
 
     std::ostringstream oss;
-    StorageBase::SaveResult saveResult = StorageBase::SaveResult::FAILED;
+    StorageBase::SaveResult saveResult(StorageBase::SaveResult::FAILED);
     try
     {
         std::unique_ptr<Poco::Net::HTTPClientSession> psession(getHTTPClientSession(uriObject));
@@ -748,7 +748,7 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization&
         std::istream& rs = psession->receiveResponse(response);
         Poco::StreamCopier::copyStream(rs, oss);
 
-        std::string wopiLog(isSaveAs? "WOPI::PutFileRelative": "WOPI::PutFile");
+        std::string wopiLog(isSaveAs? "WOPI::PutRelativeFile": "WOPI::PutFile");
         LOG_INF(wopiLog << " response: " << oss.str());
         LOG_INF(wopiLog << " uploaded " << size << " bytes from [" << filePath <<
                 "] -> [" << uriObject.toString() << "]: " <<
@@ -756,7 +756,7 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization&
 
         if (response.getStatus() == Poco::Net::HTTPResponse::HTTP_OK)
         {
-            saveResult = StorageBase::SaveResult::OK;
+            saveResult.setResult(StorageBase::SaveResult::OK);
             Poco::JSON::Object::Ptr object;
             if (parseJSON(oss.str(), object))
             {
@@ -764,38 +764,49 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization&
                 LOG_TRC(wopiLog << " returns LastModifiedTime [" << lastModifiedTime << "].");
                 _fileInfo._modifiedTime = iso8601ToTimestamp(lastModifiedTime);
 
+                if (isSaveAs)
+                {
+                    const std::string name = getJSONValue<std::string>(object, "Name");
+                    LOG_TRC(wopiLog << " returns Name [" << name << "].");
+
+                    const std::string url = getJSONValue<std::string>(object, "Url");
+                    LOG_TRC(wopiLog << " returns Url [" << url << "].");
+
+                    saveResult.setSaveAsResult(name, url);
+                }
+
                 // Reset the force save flag now, if any, since we are done saving
                 // Next saves shouldn't be saved forcefully unless commanded
                 _forceSave = false;
             }
             else
             {
-                LOG_WRN("Invalid/Missing JSON found in " << wopiLog << " response");
+                LOG_WRN("Invalid or missing JSON in " << wopiLog << " HTTP_OK response");
             }
         }
         else if (response.getStatus() == Poco::Net::HTTPResponse::HTTP_REQUESTENTITYTOOLARGE)
         {
-            saveResult = StorageBase::SaveResult::DISKFULL;
+            saveResult.setResult(StorageBase::SaveResult::DISKFULL);
         }
         else if (response.getStatus() == Poco::Net::HTTPResponse::HTTP_UNAUTHORIZED)
         {
-            saveResult = StorageBase::SaveResult::UNAUTHORIZED;
+            saveResult.setResult(StorageBase::SaveResult::UNAUTHORIZED);
         }
         else if (response.getStatus() == Poco::Net::HTTPResponse::HTTP_CONFLICT)
         {
-            saveResult = StorageBase::SaveResult::CONFLICT;
+            saveResult.setResult(StorageBase::SaveResult::CONFLICT);
             Poco::JSON::Object::Ptr object;
             if (parseJSON(oss.str(), object))
             {
                 const unsigned loolStatusCode = getJSONValue<unsigned>(object, "LOOLStatusCode");
                 if (loolStatusCode == static_cast<unsigned>(LOOLStatusCode::DOC_CHANGED))
                 {
-                    saveResult = StorageBase::SaveResult::DOC_CHANGED;
+                    saveResult.setResult(StorageBase::SaveResult::DOC_CHANGED);
                 }
             }
             else
             {
-                LOG_WRN("Invalid/missing JSON in " << wopiLog << " response");
+                LOG_WRN("Invalid or missing JSON in " << wopiLog << " HTTP_CONFLICT response");
             }
         }
     }
@@ -803,7 +814,7 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Authorization&
     {
         LOG_ERR("Cannot save file to WOPI storage uri [" + uriObject.toString() + "]. Error: " << pexc.displayText() <<
                 (pexc.nested() ? " (" + pexc.nested()->displayText() + ")" : ""));
-        saveResult = StorageBase::SaveResult::FAILED;
+        saveResult.setResult(StorageBase::SaveResult::FAILED);
     }
 
     return saveResult;
@@ -819,7 +830,7 @@ std::string WebDAVStorage::loadStorageFileToLocal(const Authorization& /*auth*/)
 StorageBase::SaveResult WebDAVStorage::saveLocalFileToStorage(const Authorization& /*auth*/, const std::string& /*saveAsPath*/, const std::string& /*saveAsFilename*/)
 {
     // TODO: implement webdav PUT.
-    return StorageBase::SaveResult::OK;
+    return StorageBase::SaveResult(StorageBase::SaveResult::OK);
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp
index 60c921913..52df9d141 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -53,14 +53,53 @@ public:
         size_t _size;
     };
 
-    enum class SaveResult
+    class SaveResult
     {
-        OK,
-        DISKFULL,
-        UNAUTHORIZED,
-        DOC_CHANGED, /* Document changed in storage */
-        CONFLICT,
-        FAILED
+    public:
+        enum Result
+        {
+            OK,
+            DISKFULL,
+            UNAUTHORIZED,
+            DOC_CHANGED, /**< Document changed in storage */
+            CONFLICT,
+            FAILED
+        };
+
+        SaveResult(Result result) : _result(result)
+        {
+        }
+
+        void setResult(Result result)
+        {
+            _result = result;
+        }
+
+        Result getResult() const
+        {
+            return _result;
+        }
+
+        void setSaveAsResult(const std::string& name, const std::string& url)
+        {
+            _saveAsName = name;
+            _saveAsUrl = url;
+        }
+
+        const std::string& getSaveAsName() const
+        {
+            return _saveAsName;
+        }
+
+        const std::string& getSaveAsUrl() const
+        {
+            return _saveAsUrl;
+        }
+
+    private:
+        Result _result;
+        std::string _saveAsName;
+        std::string _saveAsUrl;
     };
 
     enum class LOOLStatusCode
diff --git a/wsd/protocol.txt b/wsd/protocol.txt
index 25805b301..3af10f644 100644
--- a/wsd/protocol.txt
+++ b/wsd/protocol.txt
@@ -112,6 +112,8 @@ saveas url=<url> format=<format> options=<options>
 
     <url> is a URL, encoded. <format> is also URL-encoded, i.e. spaces as %20 and it can be empty
     options are the whole rest of the line, not URL-encoded, and can be empty
+    If <url> uses 'wopi:' as the protocol, the path is treated as the path on
+    the wopi host.
 
 selecttext type=<type> x=<x> y=<y>
 
@@ -310,6 +312,12 @@ pong rendercount=<num>
     sent in reply to a 'ping' message, where <num> is the total number
     of rendered tiles of the document.
 
+saveas: url=<url> name=<name>
+
+    <url> is a wopi URL of the newly created file, including the access token.
+    <name> is the resulting name (without path) that was created on the wopi
+    host. It can differ from what was requested in case the file already existed.
+
 status: type=<typeName> parts=<numberOfParts> current=<currentPartNumber> width=<width> height=<height> viewid=<viewId> [partNames]
 
     <typeName> is 'text, 'spreadsheet', 'presentation', 'drawing' or 'other. Others are numbers.
@@ -453,10 +461,11 @@ nextmessage: size=<upperlimit>
     one doesn't need to use a pre-allocated buffer when receiving
     WebSocket messages, this will go away.
 
-saveas: url=<url>
+saveas: url=<url> filename=<filename>
 
     <url> is a URL of the destination, encoded. Sent from the child to the
     parent after a saveAs() completed.
+    <filename> is the resulting jailed filename of the newly created file.
 
 client-<sessionId> <Payload Message>