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

oc_properties must use fileid instead of path #25563

Closed
PVince81 opened this issue Jul 22, 2016 · 8 comments · Fixed by #27005
Closed

oc_properties must use fileid instead of path #25563

PVince81 opened this issue Jul 22, 2016 · 8 comments · Fixed by #27005

Comments

@PVince81
Copy link
Contributor

PVince81 commented Jul 22, 2016

The oc_properties table contains any custom Webdav properties that might have been set by third party clients.

The trouble is that this table contains the full path to the target file and doesn't properly detect renames... so properties can get lost.
Furthermore, the column is too small and longer paths might not fit.

To be able to better preserve properties, we should make this table use the fileid instead of the path.

@DeepDiver1975

@PVince81
Copy link
Contributor Author

PVince81 commented Dec 9, 2016

The code in question is here: https://github.com/owncloud/core/blob/master/apps/dav/lib/Files/CustomPropertiesBackend.php

The tricky part is migrating from the old structure to the new one, basically trying to identify the files by path to find their ids. I suspect that in many envs entries are orphaned already, so need to clean those up.

@VicDeo
Copy link
Member

VicDeo commented Jan 5, 2017

@PVince81 should I keep old fields in DB for migration marking them as deprecated for the next major version or keep the whole table and populate existing entries into a new table as a repair step?

@PVince81
Copy link
Contributor Author

PVince81 commented Jan 9, 2017

Is it cheaper to create a new table than migrating the existing table and deleting the column ?

I'd say that we migrate the data to the new structure and delete the old structure once we're done. No deprecation.

Note that in 10.0 we have Doctrine migrations, so you could/should use that instead of repair steps when doing the migration.

CC @DeepDiver1975 @butonic

@VicDeo
Copy link
Member

VicDeo commented Jan 9, 2017

@PVince81 >in 10.0 we have Doctrine migrations
it is still under development, isn't it?

@PVince81
Copy link
Contributor Author

No, it already works. But core doesn't have a single migration checked in yet.
I've used it in the customgroups app for database initialization: https://github.com/owncloud/customgroups/tree/master/appinfo/Migrations

@VicDeo
Copy link
Member

VicDeo commented Jan 16, 2017

@PVince81 @DeepDiver1975 any reason why we have oc_properties table in core db_structure.xml and not in app/dav/appinfo/database.xml?

@PVince81
Copy link
Contributor Author

We probably never moved it when creating the "dav" app.

@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.