Skip to content

Conversation

@CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented Oct 8, 2025

Summary

Slight API changes:

  • FileInfo::getId and Node::getId now both returns an int|null. Previously Node::getId did return -1, while FileInfo::getId did return null for invalid files. Since Node and FileInfo were used interchangeably in some places, this would potentially cause issues.
  • FileInfo::getChecksum was documented as returning a string but would from time to time returns null. Now it only return a string or an empty string. The places were we were checking for returning null or empty string, are now only checking for empty string
  • LazyFolder now doesn't have any method related to IRootFolder, these are now exclusively in LazyRoot
  • FileInfo requires a valid path, IMountPoint and IStorage in it's constructor. This was already the case before but not enforced and many unit tests were just passing null. There was also apps/dav/lib/Connector/Sabre/Directory.php which was a valid usecase for passing a null mountpoint and storage, which is now using a DummyMountPoint (new) and a FailedStorage

TODO

Checklist

@CarlSchwan CarlSchwan added this to the Nextcloud 33 milestone Oct 8, 2025
@CarlSchwan CarlSchwan added the 2. developing Work in progress label Oct 8, 2025
@CarlSchwan CarlSchwan self-assigned this Oct 8, 2025
@CarlSchwan CarlSchwan force-pushed the carl/node-api-typing branch 2 times, most recently from ce0b6ee to 4168379 Compare October 8, 2025 12:01
@CarlSchwan CarlSchwan force-pushed the carl/node-api-typing branch from 23a6378 to b5fce71 Compare October 8, 2025 14:12
@CarlSchwan CarlSchwan marked this pull request as ready for review October 8, 2025 15:10
@CarlSchwan CarlSchwan requested review from a team, blizzz and nickvergessen as code owners October 8, 2025 15:10
@CarlSchwan CarlSchwan requested review from ArtificialOwl, provokateurin and sorbaugh and removed request for a team October 8, 2025 15:10
@nickvergessen nickvergessen removed their request for review October 9, 2025 09:26
@CarlSchwan CarlSchwan force-pushed the carl/node-api-typing branch 3 times, most recently from eb61c2c to d5c729b Compare October 9, 2025 12:12
@CarlSchwan CarlSchwan added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 16, 2025
@CarlSchwan CarlSchwan force-pushed the carl/node-api-typing branch from 4527d3f to 75e6b3e Compare October 16, 2025 09:20
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

All in all there are way too much ?? -1 in this, and in a lot of cases it does not make sense.
Code should throw early if an id is needed but unknown, it should not try to store a -1 in pendingRename and stuff like that.

@CarlSchwan CarlSchwan force-pushed the carl/node-api-typing branch 2 times, most recently from d285f6a to 8f28035 Compare October 29, 2025 10:17
@CarlSchwan CarlSchwan force-pushed the carl/node-api-typing branch 2 times, most recently from 20555ed to 852ab03 Compare November 6, 2025 16:20
@CarlSchwan CarlSchwan force-pushed the carl/node-api-typing branch 2 times, most recently from 0cecb7a to 506f73e Compare November 6, 2025 16:34
This was referenced Jan 7, 2026
Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
Same for getETag

Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
Instead of -1

Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
@CarlSchwan CarlSchwan removed this from the Nextcloud 33 milestone Jan 12, 2026
Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
Instead of just ignoring it and possibly creating subtle bugs.

Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
@CarlSchwan CarlSchwan force-pushed the carl/node-api-typing branch from b69af83 to 56c4409 Compare January 12, 2026 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants