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

remove base64 encoding of ids #2542

Merged
merged 3 commits into from
Feb 16, 2022
Merged

remove base64 encoding of ids #2542

merged 3 commits into from
Feb 16, 2022

Conversation

micbar
Copy link
Member

@micbar micbar commented Feb 15, 2022

Change: Unify file IDs

We changed the file IDs to be consistent across all our APIs (WebDAV, LibreGraph, OCS). We removed the base64 encoding. Now they are formatted like <storageID>!<opaqueID>. They are using a reserved character ! as a URL safe separator.

Related Issue

ocis PR owncloud/ocis#3185

Examples

curl -L -X PROPFIND 'https://localhost:9200/dav/spaces/6afcb97a-e632-447d-b832-1334a8a9634f' \
-H 'Depth: infinity' \
-H 'Authorization: Basic YWRtaW46YWRtaW4='
<d:multistatus xmlns:s="http://sabredav.org/ns" xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns">
    <d:response>
        <d:href>/dav/spaces/6afcb97a-e632-447d-b832-1334a8a9634f/</d:href>
        <d:propstat>
            <d:prop>
                <oc:id>6afcb97a-e632-447d-b832-1334a8a9634f!6afcb97a-e632-447d-b832-1334a8a9634f</oc:id>
                <oc:fileid>6afcb97a-e632-447d-b832-1334a8a9634f!6afcb97a-e632-447d-b832-1334a8a9634f</oc:fileid>
                <d:getetag>&#34;55e95f454e48c0047d901686f4f7027c&#34;</d:getetag>
                <oc:permissions>RDNVCK</oc:permissions>
                <d:resourcetype>
                    <d:collection/>
                </d:resourcetype>
                <oc:size>2019472</oc:size>
                <d:getlastmodified>Tue, 15 Feb 2022 11:42:10 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>/dav/spaces/6afcb97a-e632-447d-b832-1334a8a9634f/Space_Image.jpg</d:href>
        <d:propstat>
            <d:prop>
                <oc:id>6afcb97a-e632-447d-b832-1334a8a9634f!83a2c6ab-19f2-4f51-b3cb-41aef54ee5ab</oc:id>
                <oc:fileid>6afcb97a-e632-447d-b832-1334a8a9634f!83a2c6ab-19f2-4f51-b3cb-41aef54ee5ab</oc:fileid>
                <d:getetag>&#34;0440204b65664c18fa73a40cfe625815&#34;</d:getetag>
                <oc:permissions>RDNVW</oc:permissions>
                <d:resourcetype></d:resourcetype>
                <d:getcontentlength>2019367</d:getcontentlength>
                <d:getcontenttype>image/jpeg</d:getcontenttype>
                <d:getlastmodified>Tue, 15 Feb 2022 11:42:10 GMT</d:getlastmodified>
                <oc:checksums>
                    <oc:checksum>SHA1:439c4a6de0489288bb88914913fd1e5ece7d1764 MD5:2540f4e8f934296b617973f1a5cd5ae3 ADLER32:1c056526</oc:checksum>
                </oc:checksums>
                <oc:favorite>0</oc:favorite>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
    </d:response>
    <d:response>
        <d:href>/dav/spaces/6afcb97a-e632-447d-b832-1334a8a9634f/NewFile5.md</d:href>
        <d:propstat>
            <d:prop>
                <oc:id>6afcb97a-e632-447d-b832-1334a8a9634f!44b8c8ba-8f57-452f-9b28-85b2058bdac8</oc:id>
                <oc:fileid>6afcb97a-e632-447d-b832-1334a8a9634f!44b8c8ba-8f57-452f-9b28-85b2058bdac8</oc:fileid>
                <d:getetag>&#34;6f35041017b213007453cd8d74ccb331&#34;</d:getetag>
                <oc:permissions>RDNVW</oc:permissions>
                <d:resourcetype></d:resourcetype>
                <d:getcontentlength>105</d:getcontentlength>
                <d:getcontenttype>text/markdown</d:getcontenttype>
                <d:getlastmodified>Tue, 15 Feb 2022 11:41:34 GMT</d:getlastmodified>
                <oc:checksums>
                    <oc:checksum>SHA1:081272bd25665a1a884fd50f3143220411c6e8a3 MD5:9bdb7fc99749fb9b2207358aec781742 ADLER32:dd6928f0</oc:checksum>
                </oc:checksums>
                <oc:favorite>0</oc:favorite>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
    </d:response>
    <d:response>
        <d:href>/dav/spaces/6afcb97a-e632-447d-b832-1334a8a9634f/NewFolder/</d:href>
        <d:propstat>
            <d:prop>
                <oc:id>6afcb97a-e632-447d-b832-1334a8a9634f!dc4399c7-1fed-46e1-b81d-36c3db1c74ba</oc:id>
                <oc:fileid>6afcb97a-e632-447d-b832-1334a8a9634f!dc4399c7-1fed-46e1-b81d-36c3db1c74ba</oc:fileid>
                <d:getetag>&#34;0e0ec707e062c806e5e48faefcdca725&#34;</d:getetag>
                <oc:permissions>RDNVCK</oc:permissions>
                <d:resourcetype>
                    <d:collection/>
                </d:resourcetype>
                <oc:size>0</oc:size>
                <d:getlastmodified>Tue, 15 Feb 2022 11:40:51 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>

@micbar micbar force-pushed the fix-id branch 2 times, most recently from e6253f4 to 06c0c28 Compare February 15, 2022 09:59
@phil-davis
Copy link
Contributor

Note: core PR owncloud/core#39751 added the code to find the personal space id of a user. It reverse-engineers the base64-encoding and expects the ":" between parts. I expect that we need to adjust that code, put it in a branch and PR to core master, then point to that from this PR so that tests will run better.

@micbar
Copy link
Member Author

micbar commented Feb 15, 2022

@phil-davis Can we create a branch in oc10 where we change the discovery? We need to know that my changes here do not break stuff.

@phil-davis
Copy link
Contributor

@phil-davis Can we create a branch in oc10 where we change the discovery? We need to know that my changes here do not break stuff.

owncloud/core#39793 core branch update-getPersonalSpaceIdForUser commit id b8e7eb936d6648db43eb59ce171f10e14af90320

Put that in .drone.env and see what happens to CI.

@micbar
Copy link
Member Author

micbar commented Feb 15, 2022

@phil-davis I broke some move tests. Is that a problem with the expected failures or are they real failures?

@phil-davis
Copy link
Contributor

@phil-davis I broke some move tests. Is that a problem with the expected failures or are they real failures?

Those are core test scenario changes that are in core master and need a commit id bump, and the line numbers... sorting out. I will see if @kiranparajuli589 has time to do that today, and then you will be able to rebase to fix that.

@kiranparajuli589
Copy link
Contributor

kiranparajuli589 commented Feb 15, 2022

@phil-davis I broke some move tests. Is that a problem with the expected failures or are they real failures?

Those are core test scenario changes that are in core master and need a commit id bump, and the line numbers... sorting out. I will see if @kiranparajuli589 has time to do that today, and then you will be able to rebase to fix that.

@phil-davis @micbar #2545 is created to bump the latest commit id from core in reva-edge.

Should we be using the commit id from the branch update-getPersonalSpaceIdForUser?

@kiranparajuli589
Copy link
Contributor

hmm...some difference in apiWebdavMove2 features
https://drone.cernbox.cern.ch/cs3org/reva/5680/14/6

@phil-davis
Copy link
Contributor

Should we be using the commit id from the branch update-getPersonalSpaceIdForUser?

no, just do an ordinary commit id bump in a separate PR. Then this PR can be rebased.

@phil-davis
Copy link
Contributor

PR #2545 has been merged and bumped the core commit id to the current tip of master.

I rebased core PR owncloud/core#39793 and it is now core cranch update-getPersonalSpaceIdForUser with commit 10f3f9aaf4bdd80e408b055e467a167f1cb86228

@micbar you can rebase this PR, and put the above in .drone.env and maybe it will pass.

@micbar micbar marked this pull request as ready for review February 16, 2022 17:08
@cs3org cs3org deleted a comment from update-docs bot Feb 16, 2022
@butonic butonic merged commit a765314 into cs3org:edge Feb 16, 2022
@butonic butonic deleted the fix-id branch February 16, 2022 20:10
dragotin pushed a commit to butonic/reva that referenced this pull request Feb 17, 2022
* remove base64 encoding of ids

* use resourceid wrapper

* Update changelog/unreleased/remove-base64-encoding-of-ids.md

Co-authored-by: Jörn Friedrich Dreyer <[email protected]>
dragotin added a commit to butonic/reva that referenced this pull request Feb 17, 2022
@kobergj kobergj mentioned this pull request Dec 19, 2022
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.

4 participants