Skip to content

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

Merged
susnux merged 1 commit into
masterfrom
unified-sharing-share-instead-id
Aug 11, 2026
Merged

refactor: use Share for most SharingManager parameters instead of just the id#63140
susnux merged 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

As part of making this work properly, default values for permissions/properties are now applied once a source/recipient is added instead of only when listing shares.

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 3 times, most recently from bfd5bfd to c21d453 Compare August 11, 2026 01:04
@come-nc

This comment was marked as outdated.

@icewind1991
icewind1991 force-pushed the unified-sharing-share-instead-id branch 2 times, most recently from c85f755 to 567422f Compare August 11, 2026 14:24
…t the id

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

# Conflicts:
#	tests/lib/Sharing/AbstractSharingManagerTests.php

# Conflicts:
#	apps/sharing/tests/Controller/ApiV1ControllerTest.php
#	tests/lib/Sharing/AbstractSharingManagerTests.php
@icewind1991
icewind1991 force-pushed the unified-sharing-share-instead-id branch from 567422f to 55e1270 Compare August 11, 2026 14:40
@susnux
susnux merged commit 11ef574 into master Aug 11, 2026
216 checks passed
@susnux
susnux deleted the unified-sharing-share-instead-id branch August 11, 2026 16:12
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.

4 participants