Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[full-ci] Implement persistent major version workflow (v2) #40531

Merged
merged 31 commits into from
Feb 10, 2023

Conversation

mrow4a
Copy link
Contributor

@mrow4a mrow4a commented Dec 11, 2022

Description

Remark: For reviews, please keep in mind that this code is many years old with lots of legacies, I tried as much as I can not to refactor the code too much in order to avoid regressions and fit this change into OC10 release according to semver.

This PR introduces the following enhancements:

  • Change no. 1: Restore operation logic changed. Now restore is creating new current version of the file from one of past noncurrent versions of the file. Current version also receives incremented mtime for the file, and author of the files is the user that restored the file. The old noncurrent version is no longer removed upon restore and current version no longer receives mtime of the version.
  • Change no. 2: The current version of the file is now shown in the Versions Tab, highlighted with "gray" background

Screenshot 2023-01-04 at 15 32 58

  • Change no. 3: Versions now persist additional extended metadata on versioning tags, that allow easier identification of the versions. Each update increases minor version for the file

Screenshot 2023-01-04 at 15 34 50

  • Change no. 4: Current version of the file now can be published, which increases major version tag.
    Screenshot 2023-01-04 at 15 37 46

  • Change no. 5: Each new edit of the file would create noncurrent versions. The ones tagged with major version due to publishing, will be persisted long term and it wont be subject to any retention policies
    Screenshot 2023-01-04 at 15 33 33

Related Issue

How Has This Been Tested?

  • manually in the developer instance
    • test with normal fs for file and folder
    • test with normal fs for shared file file and folder
    • test with trashbin for file and folder
    • test with objectstore (it should be properly disabled)
    • test with encryption (metafiles are plaintext, files/versions encrypted)
  • unit/acceptance tests

Enable the feature and test generating new versions e.g. in shared folder on a document with TextEditor/OnlyOffice/Collabora

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

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

Example

  • User test1 creates a file shared/nest/test1.txt, version tab shows current label on the latest version
  • User test2 updates a file shared/nest/test1.txt two times, that creates two noncurrent versions
  • User test2 publishes latest version shared/nest/test1.txt , and thus would prevent expiry in the future. File is now marked with persistent label
  • Two more edits happen that increment the version tag

Screenshot 2023-01-04 at 15 09 17

@mrow4a mrow4a self-assigned this Dec 11, 2022
@update-docs
Copy link

update-docs bot commented Dec 11, 2022

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.

@mrow4a mrow4a force-pushed the feature/persistent-major-versions-v2 branch from 535b1ae to 87a9f50 Compare December 16, 2022 09:01
@mrow4a mrow4a force-pushed the feature/persistent-major-versions-v2 branch 2 times, most recently from fa544e8 to dcc11e4 Compare January 3, 2023 12:45
@owncloud owncloud deleted a comment from ownclouders Jan 3, 2023
@mrow4a mrow4a force-pushed the feature/persistent-major-versions-v2 branch from dcc11e4 to 6a89560 Compare January 3, 2023 12:53
@ownclouders
Copy link
Contributor

ownclouders commented Jan 3, 2023

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

https://drone.owncloud.com/owncloud/core/37534/143

@mrow4a mrow4a force-pushed the feature/persistent-major-versions-v2 branch 3 times, most recently from 55478be to db9c74b Compare January 4, 2023 14:02
@mrow4a mrow4a marked this pull request as ready for review January 4, 2023 14:46
@mrow4a mrow4a changed the title WIP: Implement persistent major version workflow (v2) Implement persistent major version workflow (v2) Jan 4, 2023
@mrow4a mrow4a requested a review from DeepDiver1975 January 4, 2023 14:46
@mrow4a mrow4a force-pushed the feature/persistent-major-versions-v2 branch from 4759a06 to f1ccbce Compare January 4, 2023 17:09
@owncloud owncloud deleted a comment from ownclouders Jan 5, 2023
@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/37534/9/7

There were 2 failures:

1) OCA\Files_Versions\Tests\VersioningTest::testExpire with data set "metaEnabled" (true)
Failed asserting that 3 matches expected 4.

/drone/src/apps/files_versions/tests/VersioningTest.php:354
phpvfscomposer:///drone/src/lib/composer/phpunit/phpunit/phpunit:97

2) OCA\Files_Versions\Tests\VersioningTest::testIsPublishedVersion with data set "metaEnabled" (true)
Failed asserting that false is true.

/drone/src/apps/files_versions/tests/VersioningTest.php:409
phpvfscomposer:///drone/src/lib/composer/phpunit/phpunit/phpunit:97

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/37534/143/18

runsh: Total unexpected failed scenarios throughout the test run:
webUITags/editTags.feature:15
webUITags/editTags.feature:28
    And the user edits the tag with name "random" and sets its name to "random-big" using the webUI # WebUITagsContext::theUserEditsTheTagWithNameAndSetsItsNameToUsingTheWebui()
      Page\FilesPageElement\DetailsDialog::renameTagxpath: //input[@id='view12-rename-input'] could not find tag edit input (SensioLabs\Behat\PageObjectExtension\PageObject\Exception\ElementNotFoundException)

@phil-davis
Copy link
Contributor

By default, CI gets killed as soon as one pipeline fails.
I suggest that you add [full-ci] to the PR title. Then all pipelines will run to completion and you can see all the things that are failing.

@mrow4a mrow4a changed the title Implement persistent major version workflow (v2) [full-ci] Implement persistent major version workflow (v2) Jan 5, 2023
@mrow4a mrow4a force-pushed the feature/persistent-major-versions-v2 branch 2 times, most recently from 6cdf487 to 91acbb6 Compare January 5, 2023 20:18
@mrow4a
Copy link
Contributor Author

mrow4a commented Jan 5, 2023

@phil-davis would you mind having a look into https://drone.owncloud.com/owncloud/core/37551/142/17 ? I quite cannot scratch my head what is the problem, my change has nothing to do with Tags Tab, it touches only Versions Tab. Also backend logic nowhere touches tags logic. Would be quite grateful 🙏🏻

can this be somehow related to 22985c0#diff-13542e9bb43fdf30fda1d9e9c04815936a6b19e6ab005b5c444c740f8e66646d (which is related to this exact feature) ?

@mrow4a mrow4a force-pushed the feature/persistent-major-versions-v2 branch from 91acbb6 to 32e04a5 Compare January 7, 2023 18:39
@mrow4a
Copy link
Contributor Author

mrow4a commented Jan 8, 2023

@phil-davis started adjusting other acceptance tests due to the fact that PROPFIND now also returns current version and noncurrent version, plus restore logic changed.

And... found out that in one case there is a bug in my implementation (enable file versioning and check the history of changes from multiple users after renaming file by sharer) ... without these tests would never have thought of that corner case, good job!

@mrow4a mrow4a force-pushed the feature/persistent-major-versions-v2 branch 4 times, most recently from 93e183c to 2a75559 Compare January 8, 2023 16:27
@mrow4a mrow4a force-pushed the feature/persistent-major-versions-v2 branch from 5cc9921 to a36fb61 Compare February 10, 2023 12:54
@mrow4a mrow4a requested a review from jvillafanez February 10, 2023 12:57
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 50 Code Smells

60.8% 60.8% Coverage
3.7% 3.7% Duplication

Copy link
Member

@jvillafanez jvillafanez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might need a squash, but code looks ok.

@mmattel
Copy link
Contributor

mmattel commented Feb 10, 2023

@mrow4a pls add the change that you proposed in docs in config_sample_php_parameters here in core. I will then do a config-to-docs run post merging this PR which transports the changes to docs properly. Also see my comment in the docs PR you created (thanks for creating it!).

@jnweiger
Copy link
Contributor

Confirmed fixed in 10.12.0-rc.3

  • Persistent versions are retained with auto, other versions disapear as per policy.
  • Persistent versions older than 2 days are retained with 'auto, 2', when occ versions:expire is run.
  • Persistent versions are deleted by 'occ versions:cleanup' - which is expected and documented behaviour.
  • Works fine also with encryption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants