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.

Sign up for GitHub

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

Jump to bottom

Version count is 1 more than on oC10 #1633

Closed
phil-davis opened this issue Feb 12, 2021 · 5 comments
Closed

Version count is 1 more than on oC10 #1633

phil-davis opened this issue Feb 12, 2021 · 5 comments
Labels
Type:Bug

Comments

@phil-davis
Copy link
Contributor

Describe the bug

On oC10 the count of versions of a file is just the number of "old" versions that exist.
On OCIS the count includes the current "version".

It's arguable which is "correct", but they should not be different.

Steps to reproduce

https://drone.cernbox.cern.ch/cs3org/reva/567/18/7

  Scenario: Receiver tries get file versions of shared file from the sharer                # /drone/src/tmp/testrunner/tests/acceptance/features/apiVersions/fileVersions.feature:373
    Given user "Brian" has been created with default attributes and without skeleton files # FeatureContext::userHasBeenCreatedWithDefaultAttributesAndWithoutSkeletonFiles()
    And user "Alice" has uploaded file with content "textfile0" to "textfile0.txt"         # FeatureContext::userHasUploadedAFileWithContentTo()
    And user "Alice" has uploaded file with content "version 1" to "textfile0.txt"         # FeatureContext::userHasUploadedAFileWithContentTo()
    And user "Alice" has uploaded file with content "version 2" to "textfile0.txt"         # FeatureContext::userHasUploadedAFileWithContentTo()
    And user "Alice" has uploaded file with content "version 3" to "textfile0.txt"         # FeatureContext::userHasUploadedAFileWithContentTo()
    And user "Alice" has shared file "textfile0.txt" with user "Brian"                     # FeatureContext::userHasSharedFileWithUserUsingTheSharingApi()
    When user "Brian" tries to get versions of file "textfile0.txt" from "Alice"           # FeatureContext::userTriesToGetFileVersions()
    Then the HTTP status code should be "207"                                              # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And the number of versions should be "3"                                               # FeatureContext::theNumberOfVersionsShouldBe()
      Expected number of versions was '3', but got '4'
      Failed asserting that 4 matches expected '3'.

Expected behavior

After uploading a file, then overwriting it 3 trimes, the count of versions in the version history should be 3

Actual behavior

On OCIS the count is 4. (It includes the "current version")

Setup

Current CI
@phil-davis phil-davis mentioned this issue Feb 22, 2021
Closed
@wkloucek
Copy link
Contributor

wkloucek commented Mar 9, 2021 •
edited
Loading

@butonic I'm not sure if this really needs a fix in the implementation.

The test counts the number of getlastmodfied properties:

$xmlPart = $resXml->xpath("//d:getlastmodified");
$actualNumber = \count($xmlPart);

This seems to be the problem, because oCIS has a modified date on the current version / first element too.

<?xml version="1.0" encoding="utf-8"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">
    <d:response>
        <d:href>/remote.php/dav/meta/MTI4NGQyMzgtYWE5Mi00MmNlLWJkYzQtMGIwMDAwMDA5MTU3OjY2MTMwOGFjLTk1OTctNDBiNy1hMWVjLWVjYTdiYjcwZmI3ZQ%3d%3d/v/</d:href>
        <d:propstat>
            <d:prop>
                <oc:id>dmlydHVhbDptZXRhL01USTROR1F5TXpndFlXRTVNaTAwTW1ObExXSmtZelF0TUdJd01EQXdNREE1TVRVM09qWTJNVE13T0dGakxUazFPVGN0TkRCaU55MWhNV1ZqTFdWallUZGlZamN3Wm1JM1pRPT0vdg==</oc:id>
                <oc:fileid>dmlydHVhbDptZXRhL01USTROR1F5TXpndFlXRTVNaTAwTW1ObExXSmtZelF0TUdJd01EQXdNREE1TVRVM09qWTJNVE13T0dGakxUazFPVGN0TkRCaU55MWhNV1ZqTFdWallUZGlZamN3Wm1JM1pRPT0vdg==</oc:fileid>
                <d:getetag>&#34;b65cf0e877e46bd70ba9ba3aa42e4c94&#34;</d:getetag>
                <d:resourcetype>
                    <d:collection />
                </d:resourcetype>
                <oc:size>0</oc:size>
                <d:getlastmodified>Tue, 09 Mar 2021 13:02:15 GMT</d:getlastmodified>
                <oc:favorite>0</oc:favorite>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
    </d:response>
    <d:response>
        <d:href>/remote.php/dav/meta/MTI4NGQyMzgtYWE5Mi00MmNlLWJkYzQtMGIwMDAwMDA5MTU3OjY2MTMwOGFjLTk1OTctNDBiNy1hMWVjLWVjYTdiYjcwZmI3ZQ%3d%3d/v/661308ac-9597-40b7-a1ec-eca7bb70fb7e.REV.2021-03-09T13:01:58.615289046Z</d:href>
        <d:propstat>
            <d:prop>
                <oc:id>dmVyc2lvbnM6NjYxMzA4YWMtOTU5Ny00MGI3LWExZWMtZWNhN2JiNzBmYjdlQDY2MTMwOGFjLTk1OTctNDBiNy1hMWVjLWVjYTdiYjcwZmI3ZS5SRVYuMjAyMS0wMy0wOVQxMzowMTo1OC42MTUyODkwNDZa</oc:id>
                <oc:fileid>dmVyc2lvbnM6NjYxMzA4YWMtOTU5Ny00MGI3LWExZWMtZWNhN2JiNzBmYjdlQDY2MTMwOGFjLTk1OTctNDBiNy1hMWVjLWVjYTdiYjcwZmI3ZS5SRVYuMjAyMS0wMy0wOVQxMzowMTo1OC42MTUyODkwNDZa</oc:fileid>
                <d:resourcetype></d:resourcetype>
                <d:getcontentlength>2</d:getcontentlength>
                <d:getlastmodified>Tue, 09 Mar 2021 13:01:58 GMT</d:getlastmodified>
                <oc:favorite>0</oc:favorite>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
    </d:response>
    <d:response>
        <d:href>/remote.php/dav/meta/MTI4NGQyMzgtYWE5Mi00MmNlLWJkYzQtMGIwMDAwMDA5MTU3OjY2MTMwOGFjLTk1OTctNDBiNy1hMWVjLWVjYTdiYjcwZmI3ZQ%3d%3d/v/661308ac-9597-40b7-a1ec-eca7bb70fb7e.REV.2021-03-09T13:02:02.731436422Z</d:href>
        <d:propstat>
            <d:prop>
                <oc:id>dmVyc2lvbnM6NjYxMzA4YWMtOTU5Ny00MGI3LWExZWMtZWNhN2JiNzBmYjdlQDY2MTMwOGFjLTk1OTctNDBiNy1hMWVjLWVjYTdiYjcwZmI3ZS5SRVYuMjAyMS0wMy0wOVQxMzowMjowMi43MzE0MzY0MjJa</oc:id>
                <oc:fileid>dmVyc2lvbnM6NjYxMzA4YWMtOTU5Ny00MGI3LWExZWMtZWNhN2JiNzBmYjdlQDY2MTMwOGFjLTk1OTctNDBiNy1hMWVjLWVjYTdiYjcwZmI3ZS5SRVYuMjAyMS0wMy0wOVQxMzowMjowMi43MzE0MzY0MjJa</oc:fileid>
                <d:resourcetype></d:resourcetype>
                <d:getcontentlength>2</d:getcontentlength>
                <d:getlastmodified>Tue, 09 Mar 2021 13:02:02 GMT</d:getlastmodified>
                <oc:favorite>0</oc:favorite>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
    </d:response>
    <d:response>
        <d:href>/remote.php/dav/meta/MTI4NGQyMzgtYWE5Mi00MmNlLWJkYzQtMGIwMDAwMDA5MTU3OjY2MTMwOGFjLTk1OTctNDBiNy1hMWVjLWVjYTdiYjcwZmI3ZQ%3d%3d/v/661308ac-9597-40b7-a1ec-eca7bb70fb7e.REV.2021-03-09T13:02:06.271563178Z</d:href>
        <d:propstat>
            <d:prop>
                <oc:id>dmVyc2lvbnM6NjYxMzA4YWMtOTU5Ny00MGI3LWExZWMtZWNhN2JiNzBmYjdlQDY2MTMwOGFjLTk1OTctNDBiNy1hMWVjLWVjYTdiYjcwZmI3ZS5SRVYuMjAyMS0wMy0wOVQxMzowMjowNi4yNzE1NjMxNzha</oc:id>
                <oc:fileid>dmVyc2lvbnM6NjYxMzA4YWMtOTU5Ny00MGI3LWExZWMtZWNhN2JiNzBmYjdlQDY2MTMwOGFjLTk1OTctNDBiNy1hMWVjLWVjYTdiYjcwZmI3ZS5SRVYuMjAyMS0wMy0wOVQxMzowMjowNi4yNzE1NjMxNzha</oc:fileid>
                <d:resourcetype></d:resourcetype>
                <d:getcontentlength>2</d:getcontentlength>
                <d:getlastmodified>Tue, 09 Mar 2021 13:02:06 GMT</d:getlastmodified>
                <oc:favorite>0</oc:favorite>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
    </d:response>
</d:multistatus>

Whereas oC10 doesn't have this on the first element.

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">
    <d:response>
        <d:href>/remote.php/dav/meta/24/v/</d:href>
        <d:propstat>
            <d:prop>
                <d:resourcetype>
                    <d:collection />
                </d:resourcetype>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
    </d:response>
    <d:response>
        <d:href>/remote.php/dav/meta/24/v/1615297566</d:href>
        <d:propstat>
            <d:prop>
                <d:getlastmodified>Tue, 09 Mar 2021 13:46:06 GMT</d:getlastmodified>
                <d:getcontentlength>9</d:getcontentlength>
                <d:resourcetype />
                <d:getetag>&quot;f0392ef57de1be4d9b9f39c431218a3e&quot;</d:getetag>
                <d:getcontenttype>text/plain</d:getcontenttype>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
        <d:propstat>
            <d:prop>
                <d:quota-used-bytes />
                <d:quota-available-bytes />
            </d:prop>
            <d:status>HTTP/1.1 404 Not Found</d:status>
        </d:propstat>
    </d:response>
    <d:response>
        <d:href>/remote.php/dav/meta/24/v/1615294916</d:href>
        <d:propstat>
            <d:prop>
                <d:getlastmodified>Tue, 09 Mar 2021 13:01:56 GMT</d:getlastmodified>
                <d:getcontentlength>2</d:getcontentlength>
                <d:resourcetype />
                <d:getetag>&quot;7d3cee6d0febf14b3495e8db0a915efb&quot;</d:getetag>
                <d:getcontenttype>text/plain</d:getcontenttype>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
        <d:propstat>
            <d:prop>
                <d:quota-used-bytes />
                <d:quota-available-bytes />
            </d:prop>
            <d:status>HTTP/1.1 404 Not Found</d:status>
        </d:propstat>
    </d:response>
</d:multistatus>

@butonic
Copy link
Member

butonic commented Mar 10, 2021 •
edited
Loading

hm this is a difference in the api and ocis / reva should reflect oc10. The meta node /remote.php/dav/meta/24/v/ does not really reference the file, but is just the collection of versions ... it should not have a d:getlastmodified property or any other property from the file.

https://github.com/cs3org/reva/blob/master/internal/http/services/owncloud/ocdav/versions.go#L139 should be removed, maybe all the other superflous properties as well.

@settings settings bot removed the p3-medium label Apr 7, 2021
@wkloucek
Copy link
Contributor

At least one expected failure left (https://github.com/cs3org/reva/blob/master/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md#version-count-is-1-more-than-on-oc10). This fails because of the status code and not because of the versions count.

@kobergj
Copy link
Collaborator

kobergj commented Jun 4, 2024

Seems not be a problem any more.

@kobergj kobergj closed this as completed Jun 4, 2024
@phil-davis
Copy link
Contributor Author

Seems not be a problem any more.

Correct - all the scenarios in https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiVersions/fileVersions.feature are passing nowadays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Type:Bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@butonic @phil-davis @kobergj @wkloucek