Parallel Loading of Users' Requests

[Ashod]

• wsd: exclude WopiStorage in mobile builds
• wsd: use RequestVettingStation for async loading
• wsd: simplify WebSocketHandler::sendErrorAndShutdown
• wsd: support sendMessage with string literals
• wsd: header clean up and SPDX license
• wsd: optional timeout to getHttpSession helper
• wsd: restructure RequestVettingStation interface
• wsd: use a single shared_ptr instead of container for RequestVettingStation
• wsd: extract ResourceAccessDetails from cool.html file request
• wsd: make wopiInfo a member of RequestVettingStation
• wsd: helper to create the document-load URI
• wsd: parallel load with cool.html

[mmeeks]

Ah - we should get the unit tests passing too I guess ;-)

[Ashod]

Thanks for the review and feedback!

So - overall this looks good. Seems we do the async CheckFileInfo, but don't pre-download the file (or did I miss that ? =). I prolly need to re-read / read some more I guess.

Not yet! For that we need DocBroker to get created without a client WS, create the jail, spin up a Kit, fire the poll thread, and download, while at the same time not have a client WS (it captures it in the constructor). The tricky bit is error handling, as normally we would report the error to the client and close the socket and finally disbanding the DocBroker. But if we destroyed the DocBroker without the client WS, we would re-do all this work when the client WS is created (when the browser open the WS), which is wasteful. So there is a bit of reworking that needs to be done before downloading in parallel works.

Rest assured it is in the works.

I'm worried about:

disposition.setMove - we use this a fair bit, yet:

// not the method you want.
void setMove(MoveFunction moveFn)

concerns me - I'm hazy about its usage; can you check it's the right disposition where used vs. transfer etc.

Good point. Read that patch. A thorough review is in order. Will do.

I would like a clean, single pure code-movement commit that moves

COOLWSD ClientRequestDispatcher - into its own file - COOLWSD.cpp is far too large: no other code-changes though - a pure code-move commit =)

Agreed. It has grown way too large and I wanted to move it. Perhaps a good idea to do it before this one lands.

Can you test copy/paste ?

Will do. I was relying on the unit-tests, but I think we need some manual testing too.

Otherwise - don't want to block this getting in; looks like a good direction.

I have broken this up into https://github.com/CollaboraOnline/online/pull/8179 and https://github.com/CollaboraOnline/online/pull/8180

I'll rebase when the above two land. Meanwhile, will follow up on the feedback here.

Thanks Ash! =)

Thank you!

[mmeeks]

But if we destroyed the DocBroker without the client WS, we would re-do all this work when the client WS is created (when the browser open the WS), which is wasteful. So there is a bit of reworking that needs to be done before downloading in parallel works.

I don't think we need to worry about making an error case - which after all won't do a download anyway particularly optimal. We don't want to remember a previous error I think; just re-issue the request at the later point if necessary and it failed in the background and/or is not still downloading I think =)

Can we get some more of these re-factors, SPX cleanup etc. pushed separately too ? =) Thanks

[mmeeks]

Glad this got in - good stuff =) no I guess for pipelining the download too (?) =)