deck icon indicating copy to clipboard operation
deck copied to clipboard

Optimize getSharedWith to fetch permissions with initial query instead of one per card

Open juliusknorr opened this issue 1 year ago • 6 comments

  • fix: Add method to map board to file permissions
  • perf(sharing): Optimize getSharedWith to fetch permissions right away
  • fix: Chunk query for getting labels for cards
  • Resolves: #
  • Target version: main

Summary

--- query_before	2024-07-15 20:34:24
+++ query_after	2024-07-15 20:34:06
@@ -7,7 +7,6 @@
 primary  0 SELECT `uid`, `displayname`, `password` FROM `oc_users` WHERE `uid_lower` = :dcValue1
 primary  0 SELECT `provider_id`, `enabled` FROM `oc_twofactor_providers` WHERE `uid` = :dcValue1
 primary  0 SELECT `appid`, `configkey`, `configvalue` FROM `oc_preferences` WHERE `userid` = :dcValue1
-primary  0 UPDATE `oc_preferences` SET `configvalue` = :dcValue1 WHERE (`userid` = :dcValue2) AND (`appid` = :dcValue3) AND (`configkey` = :dcValue4)
 primary  0 SELECT * FROM `oc_notifications_settings` WHERE `user_id` = :dcValue1
 primary  0 SELECT * FROM `oc_ldap_group_membership` WHERE `userid` = :dcValue1
 primary  0 SELECT `credentials` FROM `oc_storages_credentials` WHERE (`identifier` = :dcValue1) AND (`user` = :dcValue2)
@@ -34,16 +33,13 @@
 primary  0 SELECT `r`.`token` FROM `oc_talk_attendees` `a` LEFT JOIN `oc_talk_rooms` `r` ON `a`.`room_id` = `r`.`id` WHERE (`a`.`actor_id` = :dcValue1) AND (`a`.`actor_type` = :dcValue2)
 primary  0 SELECT `s`.*, `f`.`fileid`, `f`.`path`, `f`.`permissions` as `f_permissions`, `f`.`storage`, `f`.`path_hash`, `f`.`parent` as `f_parent`, `f`.`name`, `f`.`mimetype`, `f`.`mimepart`, `f`.`size`, `f`.`mtime`, `f`.`storage_mtime`, `f`.`encrypted`, `f`.`unencrypted_size`, `f`.`etag`, `f`.`checksum`, `st`.`id` AS `storage_string_id` FROM `oc_share` `s` LEFT JOIN `oc_filecache` `f` ON `s`.`file_source` = `f`.`fileid` LEFT JOIN `oc_storages` `st` ON `f`.`storage` = `st`.`numeric_id` WHERE (`s`.`share_type` = :dcValue1) AND (`s`.`share_with` IN (:dcValue2)) AND ((`s`.`item_type` = :dcValue3) OR (`s`.`item_type` = :dcValue4)) ORDER BY `s`.`id` ASC
 primary  0 SELECT * FROM `oc_share` WHERE (`share_type` = :dcValue1) AND (`share_with` = :dcValue2) AND ((`item_type` = :dcValue3) OR (`item_type` = :dcValue4))
-primary  0 SELECT DISTINCT `b`.`id` FROM `oc_deck_boards` `b` WHERE `owner` = :dcValue1
-primary  0 SELECT DISTINCT `b`.`id` FROM `oc_deck_boards` `b` INNER JOIN `oc_deck_board_acl` `acl` ON `b`.`id` = `acl`.`board_id` WHERE ((`acl`.`type` = :dcValue1) AND (`acl`.`participant` = :dcValue2)) OR ((`acl`.`type` = :dcValue3) AND (`acl`.`participant` IN (:dcValue4))) OR ((`acl`.`type` = :dcValue5) AND (`acl`.`participant` IN (:dcValue6)))
-primary  0 SELECT `s`.*, `f`.`fileid`, `f`.`path`, `f`.`permissions` as `f_permissions`, `f`.`storage`, `f`.`path_hash`, `f`.`parent` as `f_parent`, `f`.`name`, `f`.`mimetype`, `f`.`mimepart`, `f`.`size`, `f`.`mtime`, `f`.`storage_mtime`, `f`.`encrypted`, `f`.`unencrypted_size`, `f`.`etag`, `f`.`checksum`, `st`.`id` AS `storage_string_id` FROM `oc_share` `s` LEFT JOIN `oc_filecache` `f` ON `s`.`file_source` = `f`.`fileid` LEFT JOIN `oc_deck_cards` `dc` ON CAST(`dc`.`id` AS CHAR) = `s`.`share_with` LEFT JOIN `oc_storages` `st` ON `f`.`storage` = `st`.`numeric_id` LEFT JOIN `oc_deck_stacks` `ds` ON `dc`.`stack_id` = `ds`.`id` LEFT JOIN `oc_deck_boards` `db` ON `ds`.`board_id` = `db`.`id` WHERE (`s`.`share_type` = :dcValue1) AND (`db`.`id` IN (:dcValue2)) AND ((`s`.`item_type` = :dcValue3) OR (`s`.`item_type` = :dcValue4)) ORDER BY `s`.`id` ASC
+primary  0 SELECT `id`, `title`, `owner`, `color`, `archived`, `deleted_at`, `last_modified` FROM `oc_deck_boards` `b` WHERE `owner` = :dcValue1 ORDER BY `b`.`id` ASC
+primary  0 SELECT `b`.`id`, `title`, `owner`, `color`, `archived`, `deleted_at`, `last_modified` FROM `oc_deck_boards` `b` INNER JOIN `oc_deck_board_acl` `acl` ON `b`.`id` = `acl`.`board_id` WHERE (`acl`.`participant` = :dcValue2) AND (`acl`.`type` = :dcValue3) AND (`b`.`owner` <> :dcValue4) ORDER BY `b`.`id` ASC
+primary  0 SELECT `b`.`id`, `title`, `owner`, `color`, `archived`, `deleted_at`, `last_modified` FROM `oc_deck_boards` `b` INNER JOIN `oc_deck_board_acl` `acl` ON `b`.`id` = `acl`.`board_id` WHERE (`acl`.`type` = :dcValue1) AND (`b`.`owner` <> :dcValue2) AND ((`acl`.`participant` = :dcValue3) OR (`acl`.`participant` = :dcValue4)) ORDER BY `b`.`id` ASC
+primary  0 SELECT `b`.`id`, `title`, `owner`, `color`, `archived`, `deleted_at`, `last_modified` FROM `oc_deck_boards` `b` INNER JOIN `oc_deck_board_acl` `acl` ON `b`.`id` = `acl`.`board_id` WHERE (`acl`.`type` = :dcValue1) AND (`b`.`owner` <> :dcValue2) AND ((`acl`.`participant` = :dcValue3) OR (`acl`.`participant` = :dcValue4)) ORDER BY `b`.`id` ASC
+primary  0 SELECT `id`, `board_id`, `type`, `participant`, `permission_edit`, `permission_share`, `permission_manage` FROM `oc_deck_board_acl` WHERE `board_id` IN (:boardIds)
+primary  0 SELECT `s`.*, `f`.`fileid`, `f`.`path`, `f`.`permissions` as `f_permissions`, `f`.`storage`, `f`.`path_hash`, `f`.`parent` as `f_parent`, `f`.`name`, `f`.`mimetype`, `f`.`mimepart`, `f`.`size`, `f`.`mtime`, `f`.`storage_mtime`, `f`.`encrypted`, `f`.`unencrypted_size`, `f`.`etag`, `f`.`checksum`, `st`.`id` AS `storage_string_id`, `db`.`id` AS `board_id` FROM `oc_share` `s` LEFT JOIN `oc_filecache` `f` ON `s`.`file_source` = `f`.`fileid` LEFT JOIN `oc_deck_cards` `dc` ON CAST(`dc`.`id` AS CHAR) = `s`.`share_with` LEFT JOIN `oc_storages` `st` ON `f`.`storage` = `st`.`numeric_id` LEFT JOIN `oc_deck_stacks` `ds` ON `dc`.`stack_id` = `ds`.`id` LEFT JOIN `oc_deck_boards` `db` ON `ds`.`board_id` = `db`.`id` WHERE (`dc`.`deleted_at` = :dcValue1) AND (`db`.`deleted_at` = :dcValue2) AND (`s`.`share_type` = :dcValue3) AND (`db`.`id` IN (:dcValue4)) AND ((`s`.`item_type` = :dcValue5) OR (`s`.`item_type` = :dcValue6)) ORDER BY `s`.`id` ASC
 primary  0 SELECT * FROM `oc_share` WHERE (`parent` IN (:dcValue1)) AND (`share_with` = :dcValue2) AND ((`item_type` = :dcValue3) OR (`item_type` = :dcValue4))
-primary  0 SELECT * FROM `oc_deck_boards` WHERE (`id` = :dcValue2) AND (`deleted_at` = :dcValue1) ORDER BY `id` ASC
-primary  0 SELECT `id`, `board_id`, `type`, `participant`, `permission_edit`, `permission_share`, `permission_manage` FROM `oc_deck_board_acl` WHERE `board_id` = :dcValue1
-primary  0 SELECT `id`, `board_id`, `type`, `participant`, `permission_edit`, `permission_share`, `permission_manage` FROM `oc_deck_board_acl` WHERE `board_id` = :dcValue1
-primary  0 SELECT * FROM `oc_deck_cards` WHERE `id` = :dcValue1 ORDER BY `order` ASC, `id` ASC
-primary  0 SELECT * FROM `oc_deck_cards` WHERE `id` = :dcValue1 ORDER BY `order` ASC, `id` ASC
-primary  0 SELECT * FROM `oc_deck_cards` WHERE `id` = :dcValue1 ORDER BY `order` ASC, `id` ASC
 primary  0 SELECT `storage_id`, `root_id`, `user_id`, `mount_point`, `mount_id`, `mount_provider_class` FROM `oc_mounts` `m` WHERE `user_id` = ?
 primary  1 INSERT INTO `oc_mounts` (`storage_id`,`root_id`,`user_id`,`mount_point`,`mount_id`,`mount_provider_class`) SELECT ?,?,?,?,?,? FROM `oc_mounts` WHERE `root_id` = ? AND `user_id` = ? AND `mount_point` = ? HAVING COUNT(*) = 0
 primary  1 INSERT INTO `oc_mounts` (`storage_id`,`root_id`,`user_id`,`mount_point`,`mount_id`,`mount_provider_class`) SELECT ?,?,?,?,?,? FROM `oc_mounts` WHERE `root_id` = ? AND `user_id` = ? AND `mount_point` = ? HAVING COUNT(*) = 0

TODO

  • [ ] ...

Checklist

  • [ ] Code is properly formatted
  • [ ] Sign-off message is added to all commits
  • [ ] Tests (unit, integration, api and/or acceptance) are included
  • [ ] Documentation (manuals or wiki) has been updated or is not required

juliusknorr avatar Jul 15 '24 18:07 juliusknorr

@juliushaertl I think cypress/integration tests are failing. :)

luka-nextcloud avatar Sep 20 '24 18:09 luka-nextcloud

Tests should pass now.

juliusknorr avatar Dec 19 '24 16:12 juliusknorr

🐢 Performance warning. It looks like the query count of the integration tests increased with this PR. Database query count is now 71634 was 70520 (+1.57%) Please check your code again. If you added a new test this can be expected and the base value in tests/integration/base-query-count.txt can be increased.

github-actions[bot] avatar Dec 19 '24 16:12 github-actions[bot]

🐢 Performance warning. It looks like the query count of the integration tests increased with this PR. Database query count is now 71634 was 70520 (+1.57%) Please check your code again. If you added a new test this can be expected and the base value in tests/integration/base-query-count.txt can be increased.

github-actions[bot] avatar Dec 19 '24 17:12 github-actions[bot]

Query number increase is coming from the change of using BoardMapper:findAllForUser instead of BoardMapper::findBoardIds which does one more query then the other method. Screenshot 2024-12-19 at 18 24 21

Might be worth to double check, however the noticable query saving does not happen in the integration tests as we mostly share one card with an attachment to another user.

Needs checking if we can still optimize the above part

juliusknorr avatar Dec 19 '24 17:12 juliusknorr

@juliusknorr Can we get this merged soon?

luka-nextcloud avatar May 21 '25 06:05 luka-nextcloud