To comment on the following update, log in, then open the issue: http://www.openoffice.org/issues/show_bug.cgi?id=29152
------- Additional comments from [email protected] Tue Dec 1 10:48:37 +0000 2009 ------- tml wrote: >It means that the more time I spend staring in my editor or the debugger at the >code touched and added by the patch, the less I understand it, and the more >convinced I am that it "works" by accident;) I fully agree here. -) After spending some time with studying the patch (and the changes done afterwardsin CWS webdavandgvfslocking1) I got the idea, but I still have concerns re both the concept and the implementation: a) The WebDAV-UCP simply cannot safely decide on its own whether it is okay to lock the DAV resource before doing the GET. It might be okay in the context of loading a document, but not in case of loading linked graphics (like tml described above) or when doing an extension/product update ping. Well, with the patch a client might control this using the type of data sink handed over to the "open" command (it must be a data streamer in order to trigger locking, IIRC), but I consider this not optimal, despite of the current framework bugs not using the different data sinks "right". b) The patch is rather big, even after taking out the code peaces that are not related to WebDAV locking. Larger code changes in sensitive areas, spread over many modules ... c) ooo-build uses this patch for a longer time now and still has issues with this feature (see above) d) The CWS is based on an ancient OOo milestone. Resyncing will be like hell, I'm afraid. e) If I got things right, the implementation of determining whether a resource is locked and therefore shall be openened readonly is currently based of a member variable of the corresponding UCB content object. This breaks as soon as the corresponding object gets released while the document is open, which does currently not happens -- by luck. f) The current implementation contains many room for improvements (like discussed and confirmed earlier in this issue), including hacks for that we don't have a clue how to do things right. Thus, I sat together with mav (he is known to be our document load/save framework expert) and discussed the locking-topic in depth. The outcome is that we would prefer an implicit locking approach. Every UCB content implementation (not just WebDAV) could support two new commands "lock" and "unlock". "lock" creates an exclusive write lock with an indefinite lifetime. Client must call "unlock" to release the lock when it is done with the document (document gets closed). This approach has several advantages over the current (the implicit locking) approach: a) The OOo document load/save framework is the right place to decide whether a lock is needed for the document resource. All needed context is available there. b) Meanwhile (since the patch was created) the framework was greatly enhanced in a way that it is now easy to add the lock/unlock logic. c) Far less (non-UCB) code changes in less places are necessary to implement generic document locking feature for OOo. d) The new concept does not only work for WebDAV. Even the file-UCP could be extended to support the new lock/unlock commands instead of the "open" command with special arguments approach it currently supports. e) a clean implementation, without hacks should be possible (can tell this for sure for WebDAV-UCP ;-) Having all this said, I want to give you a heads up that I created issue 106830 to extend the WebDAV-UCP to support the new lock / unlock commands. Implementation makes good progress. I took as many code peaces (as well as conceptional ideas) from the original locking patch. This saved me some time. I very appreciate the work already done by the original patch contributors. Thanks a lot. Plan is to implement the necessary OOo document load/save framework changes asap. This part will be done by mav. If all works out well, we will end up with OOo 3.3 supporting WebDAV locking in a clean and maintainable way. :-) --------------------------------------------------------------------- Please do not reply to this automatically generated notification from Issue Tracker. Please log onto the website and enter your comments. http://qa.openoffice.org/issue_handling/project_issues.html#notification --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
