[WIP] Implement persistent major version workflow

[JammingBen]

Description

TODO @mrow4a

Related Issue

• https://github.com/owncloud/enterprise/issues/5286

Remaining To-Dos

• Clean Up @JammingBen
• Improve layout for current @JammingBen
• Adjust restore to create new version @JammingBen
• Bind functionally completely to the file_storage.save_version_author flag @JammingBen
• Rename file_storage.save_version_author flag @mrow4a
• Fix expiry error logs @JammingBen
• Make sure the feature is backwards compatible with past current version logic @mrow4a
• Add tests @mrow4a
• ? @mrow4a
• changelog @mrow4a

How tested

@mrow4a

• prepare env

docker run -t -d -p 8098:9980 -e "extra_params=--o:ssl.enable=false" -e "username=admin" -e "password=admin" --name collaboraocdev --cap-add MKNOD collabora/code:6.4.8.6

occ config:app:set richdocuments wopi_url --value http://localhost:8098
occ a:d files_texteditor
occ a:e testing
occ a:e richdocuments

occ config:system:set file_storage.save_version_author --value true --type boolean

export OC_PASS=test
occ user:add --email test1@test.com --password-from-env test1
occ user:add --email test2@test.com --password-from-env test2

occ group:add grp1
occ group:add-member grp1 --member test1
occ group:add-member grp1 --member test2

• create shared folder with a document, and collaborativelly edit between test1 and test2
• check versions tab of the edited file in various editing configurations, restores and publishes

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[ownclouders]

💥 Acceptance tests pipeline apiVersions-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/36722/88

[mrow4a]

@JammingBen @hodyroff FYI, I unfortunetely would need to rollback the cleanup you did for "current version" handling. There are 2 main reasons:

• I investigated and backwards compatibility with existing meta-files layout is very complicated
• In your implementation, when file is created you automatically create additional version for it, doubling the file creation latency and storage.

I think I would take this work into separate PR and start it from scratch, using your code as a base for it. Otherwise it would take too long to rollback the changes you did as part of this breaking cleanup that you did.

[JammingBen]

Sure, just take what you need 🙂 The PR is mainly a "proof of concept" - things like performance or backwards-compatibility were not taken into account here.

[mrow4a]

@JammingBen I now completely follow on your decision to create a version file on each create or update of the file, additionally to just moving old file to versioned file..

Basically the way current files_version app works, is completely not compatible with the requirements of this FR (actually also with the requirements for previous PR with "editor of the version"). Version App was designed for non-conncurrent versions of the file, which were exposed via DAV Collection, not for handling current version of file (logic from files app). The logic added recently with "filename.current.json" feels really like a hack, looking at how MetaVersionCollection over DAV is implemented.

In fact, to make this proper we would need to refactor completely entire files_version app to cover both concurrent version of the file (one in user/files folder) and non-concurrent (one in user/file_versions folder). I agree this would not happen.

I now try to use DAV PROPFIND in order to return both metadata of meta versions root ("filename.current.json") and metadata of version collection ("filename.vX.json"), using PROPFIND with different depths. This approach seems the only one to fit into existing DAV protocol for versions, and that does not lead to both backwards compatibility and performance problems desribed in #40319 (comment) .

I will send/update this PR next days with the new approach using DAV.. otherwise if I fail we need to rollback to your approach, and deprecate the behaviour with "filename.current.json" (but it will be pretty ugly because we will loose metadata for 1 of the versions and customers may complain).. will be hard to justify it as a bug, because it would be intentional regression..

[mrow4a]

@JammingBen just FYI

I found another bug, that increment algo is wrong. By just adding 0.1 you can have only 9 versions and then it goes to 1.0. It needs split into major and minor, and increment there.

Also found few bugs with restore, because when restoring we actally would mess-up counting. It should keep new version number and have a additional info that it was restored from past version number, otherwise it is super confusing in display.

[mrow4a]

closing this PR in favour of #40531 that rearchitects this feature for performance, backwards compatibility and correct handling of restore functionality