Skip to content

refactor: use Share for most SharingManager parameters instead of just the id - #63140

Open
icewind1991 wants to merge 1 commit into
masterfrom
unified-sharing-share-instead-id
Open

refactor: use Share for most SharingManager parameters instead of just the id#63140
icewind1991 wants to merge 1 commit into
masterfrom
unified-sharing-share-instead-id

Conversation

@icewind1991

@icewind1991 icewind1991 commented Aug 10, 2026

Copy link
Copy Markdown
Member

Summary

Change most methods in ISharingManager from taking an string $id to a Share $share, and return the updated share for any relevant methods.

  • Prevent unexpected behavior when calling update methods on shares that don't exist
  • Since all of those methods ended up fetching the shares anyway for processShareUpdates, having the caller fetch the share (if they don't have it already) is free.
  • We no longer need to fetch the owner from the db to perform access checks
  • We have the full share data to perform more advanced access checks in the future
  • The caller no longer needs to re-fetch the share after modifying it

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@icewind1991 icewind1991 added this to the Nextcloud 35 milestone Aug 10, 2026
@icewind1991
icewind1991 requested a review from a team as a code owner August 10, 2026 23:34
@icewind1991
icewind1991 requested review from come-nc, leftybournes, provokateurin and salmart-dev and removed request for a team August 10, 2026 23:34
@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Aug 10, 2026
@icewind1991
icewind1991 requested a review from CarlSchwan August 10, 2026 23:34
@icewind1991
icewind1991 force-pushed the unified-sharing-share-instead-id branch 2 times, most recently from 2b1b6e0 to bfd5bfd Compare August 11, 2026 00:49
…t the id

Signed-off-by: Robin Appelman <robin@icewind.nl>

# Conflicts:
#	tests/lib/Sharing/AbstractSharingManagerTests.php
@icewind1991
icewind1991 force-pushed the unified-sharing-share-instead-id branch from bfd5bfd to c21d453 Compare August 11, 2026 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant