Skip to content

Conversation

@joshtrichards
Copy link
Member

@joshtrichards joshtrichards commented Dec 16, 2025

  • Resolves: #

Summary

This updates the IMountPoint interface documentation to specify that getNumericStorageId returns -1 if the storage is unavailable, not null, aligning docs with actual implementations.

Also aligned documentation language and improved clarity throughout the interface.

Found while doing some refactoring for groupfolders.

TODO

  • ...

Checklist

…terface

This updates the IMountPoint interface documentation to specify that getNumericStorageId returns -1 if the storage is unavailable, not null, aligning docs with actual implementations.

Also aligned documentation language and improved clarity throughout the interface.

Signed-off-by: Josh <[email protected]>
Added additional details to the return annotation for clarity.

Signed-off-by: Josh <[email protected]>
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 37 to 44
/**
* Returns the storage backend mounted at this point.
*
* Result may be memoized for subsequent calls.
*
* @return \OCP\Files\Storage\IStorage|null The mounted storage backend, or null if initialization failed.
* @since 8.0.0
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There already is a comment on this method, please replace it instead of adding another one.

* This integer ID is more efficient for database operations and lookups compared
* to the string-based storage ID.
*
* @return int Numeric storage identifier, or -1 if storage cannot be initialized
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could make the type -1|positive-int

* This is the unique id (from the file cache) of the root entry for the storage mounted at this point.
* Returns -1 if the storage is not available or has not been scanned yet.
*
* @return int File id of the root folder, or -1 if unavailable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

* with string keys and mixed values.
*
* @return array
* @return array Associative array of mount options (may be empty)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be array<string, mixed>.

* such as in the case of temporary or system mounts.
*
* @return int|null mount id or null if not applicable
* @return int|null Mount point id, or null if not applicable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

positive-int|null

* The mount provider is the service or class that created and manages this mount
* (for example, a class handling user homes, external storage, or shared mounts).
*
* @return string Fully-qualified class name of the provider, or empty string if the provider is not set.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

class-string<\OCP\Files\Config\IMountProvider>

Copy link
Member

@CarlSchwan CarlSchwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice improvement :)

Comment on lines +49 to +50
* @return int The numeric storage ID from the oc_storages table,
* or -1 if the storage is unavailable or failed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @return int The numeric storage ID from the oc_storages table,
* or -1 if the storage is unavailable or failed.
* @return -1|positive-int The numeric storage ID from the oc_storages table,
* or -1 if the storage is unavailable or failed.

This was referenced Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants