fix(objectstore): do not warn when ensuring the storage root exists - #63889
Conversation
ObjectStoreStorage creates the storage root on the fly in getMetaData(), with $force = true, whenever the cache has no entry for it. A caller that then explicitly ensures the root exists - SetupManager does exactly that, once per account at first filesystem setup - is warned about a folder the storage created itself moments earlier. The message renders with an empty path, because the root normalises to the empty string. Skip the warning for the root, and quote the path so an empty one is not invisible in the log. Signed-off-by: Josua Hunziker <josh@o23.ch>
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
|
Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22 |
Fixes #63888
Summary
On instances with primary object storage, every account creation logs a
warningwhose path renders empty:ObjectStoreStoragecreates the storage root on the fly ingetMetaData(), with$force = true, whenever the cache has no entry for it:https://github.com/nextcloud/server/blob/master/lib/private/Files/ObjectStore/ObjectStoreStorage.php#L237-L239
SetupManagerthen explicitly ensures that same root exists, once per account at first filesystem setup:https://github.com/nextcloud/server/blob/master/lib/private/Files/SetupManager.php#L335
Since that call cannot pass
$force—IStorage::mkdir()declares a single parameter, and$force/$metadataexist only on theObjectStoreStorageimplementation — it takes the warning branch and reports a folder the storage created itself moments earlier. The path is the storage root, which normalises to'', hence the empty message.What this changes
Skips the warning when the path is the storage root, since an existing root is never an error, and quotes the path so an empty one is not invisible in the log.
The nested condition is deliberate, rather than extending the surrounding
ifto!$force && $path !== '' && $this->file_exists($path). That would stop the function returning early for the root, letting it fall through towhich re-mints the root's etag and mtime and flips the return from
falsetotrue— i.e. exactly what$force = truedoes, for a caller that deliberately did not pass it. Re-writing the home root's etag on each setup would risk telling sync clients the whole tree changed. Keeping the early return leaves control flow untouched and changes only what is logged.How to test
On an instance with primary object storage:
Before: one
warning-level entry per account, with an empty path.After: none. The warning still fires for any non-root folder that already exists.
Verified reproducible on 34.0.3 with S3 primary storage — the occurrence count in
nextcloud.logrose by exactly one perocc user:add.Checklist