Shares file_parent attribute is unused ?

[PVince81]

The file_parent attribute of the OCS Share seems to contain the file id of the parent folder of whatever was shared.
This is currently missing in OCIS.

The file_parent attribute doesn't seem to be used in OC 10:

apps/files_sharing/lib/ShareBackend/File.php:                           $file['parent'] = $item['file_parent'];
apps/files_sharing/lib/Controller/Share20OcsController.php:             $result['file_parent'] = $node->getParent()->getId();
core/js/shareitemmodel.js:              'id', 'file_parent', 'mail_send', 'file_source', 'item_source', 'permissions',
core/js/sharemodel.js:                  delete data.file_parent;
lib/private/Share/Share.php:                    if ($fileDependent && (int)$row['file_parent'] === -1) {
lib/private/Share/Share.php:                                    . '`*PREFIX*storages`.`id` AS `storage_id`, `*PREFIX*filecache`.`parent` as `file_parent`, '
lib/private/Share/Share.php:                                            . '`*PREFIX*storages`.`id` AS `storage_id`, `*PREFIX*filecache`.`parent` as `file_parent`';
lib/private/Share/Share.php:                                                    . '`*PREFIX*share`.`permissions`, `expiration`, `storage`, `*PREFIX*filecache`.`parent` as `file_parent`, '
lib/private/Share/Share.php:                                                    . '`*PREFIX*storages`.`id` AS `storage_id`, `*PREFIX*filecache`.`parent` as `file_parent`';
lib/private/Share/Share.php:            if (isset($row['file_parent'])) {
lib/private/Share/Share.php:                    $row['file_parent'] = (int) $row['file_parent'];

Implementing this means doing additional requests to query the parent in OCIS, but it feels like it's a waste of time and performance for an information that will not be used. If needed, a client could query the parent folder for its id.

I suspect that this is a legacy attribute that might have been used in former major versions of ownCloud.

My suggestion would be to skip implementing this and adjusting the tests accordingly, so:

• verify that all clients don't need file_parent:
  • OC 10 web UI (grepped on enterprise-complete tarball)
  • Phoenix (grepped for "file_parent" and ".parent")
  • Android
  • iOS
  • Desktop
• adjust owncloud/core API tests to skip the "file_parent" attribute in all tests, either for everything or add an exception for OCIS
• remove TODO references in OCIS code about file parent (or add a note there that it is not implemented on purpose)

@felix-schwarz @TheOneRing @hosy @individual-it @butonic @refs

[PVince81]

this is blocking the enabling of API tests for #249 and many other scenarios

[PVince81]

got confirmation that none of the clients are using this attribute

@phil-davis @individual-it possible approaches for the tests:

1. Decide that file_parent is unimportant and remove it in all the test specs: quick and easy, no code duplication
   or
2. Separate every test by cloning its scenario and in the OCIS one we remove the "file_parent" in the second one: causes scenario duplication and potentially more confusion with not much gain
   or
3. Skip the attribute in the PHP test code whenever we want to verify it: like 1 but more obscure, someone reading the test will not know that this attribute is skipped

I think I'd vote for 1 here as it's the most straightforward one.
Any objections ?

[phil-davis]

@PVince81 (1) sounds good to me - file_parent becomes a possible attribute that is not a required part of "the spec". We could just put a note/comment in one simple scenario to mention that file_parent might exist, but is not required or tested.

If we really think that there is nothing out there that uses file_parent then we could actually delete it from the server-side code. And then we really can delete every mention of it, even not make a comment in tests.

[PVince81]

I wouldn't want to risk removing it from the SQL queries in the core code as I'm unsure about side effects. Might need to evaluate whether there is a big benefit like increased performance before doing so.

[phil-davis]

I will do (1) straight away - PR incoming...

[phil-davis]

owncloud/core#37629