perf: optimize WorkspaceService::getFile

[icewind1991]

📝 Summary

Optimize WorkspaceService::getFile by removing the separate check for nodeExists

🏁 Checklist

• Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
• Sign-off message is added to all commits
• Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
• Documentation (README or documentation) has been updated or is not required

[kesselb]

What do you think about a backport?

I'm aware it's a performance optimization in the first place, but removing nodeExists is also bugfix. Folder.nodeExists uses Folder.get and is not catching all possible exceptions properly. We will send pr for server to fix Folder.nodeExists if possible.

The stack trace below is from 29.0.7 with a faulty external dav storage.

{
  "Exception": "OCP\\Files\\StorageNotAvailableException",
  "Message": "Sabre\\HTTP\\ClientHttpException: Internal Server Error",
  "Code": 1,
  "Trace": [
    {
      "file": "lib/private/Files/Storage/DAV.php",
      "line": 298,
      "function": "convertException",
      "class": "OC\\Files\\Storage\\DAV",
      "type": "->"
    },
    {
      "file": "lib/private/Files/Storage/DAV.php",
      "line": 600,
      "function": "propfind",
      "class": "OC\\Files\\Storage\\DAV",
      "type": "->"
    },
    {
      "file": "lib/private/Files/Storage/DAV.php",
      "line": 660,
      "function": "getMetaData",
      "class": "OC\\Files\\Storage\\DAV",
      "type": "->"
    },
    {
      "file": "lib/private/Files/Storage/Common.php",
      "line": 470,
      "function": "stat",
      "class": "OC\\Files\\Storage\\DAV",
      "type": "->"
    },
    {
      "file": "apps/files_sharing/lib/External/Storage.php",
      "line": 213,
      "function": "test",
      "class": "OC\\Files\\Storage\\Common",
      "type": "->"
    },
    {
      "file": "lib/private/Files/Storage/Wrapper/Wrapper.php",
      "line": 480,
      "function": "test",
      "class": "OCA\\Files_Sharing\\External\\Storage",
      "type": "->"
    },
    {
      "file": "lib/private/Files/Storage/Wrapper/Availability.php",
      "line": 69,
      "function": "test",
      "class": "OC\\Files\\Storage\\Wrapper\\Wrapper",
      "type": "->"
    },
    {
      "file": "lib/private/Files/Storage/Wrapper/Availability.php",
      "line": 83,
      "function": "updateAvailability",
      "class": "OC\\Files\\Storage\\Wrapper\\Availability",
      "type": "->",
      "args": [
        "*** sensitive parameters replaced ***"
      ]
    },
    {
      "file": "lib/private/Files/Storage/Wrapper/Availability.php",
      "line": 92,
      "function": "isAvailable",
      "class": "OC\\Files\\Storage\\Wrapper\\Availability",
      "type": "->"
    },
    {
      "file": "lib/private/Files/Storage/Wrapper/Availability.php",
      "line": 242,
      "function": "checkAvailability",
      "class": "OC\\Files\\Storage\\Wrapper\\Availability",
      "type": "->"
    },
    {
      "file": "lib/private/Files/Storage/Wrapper/Wrapper.php",
      "line": 233,
      "function": "file_exists",
      "class": "OC\\Files\\Storage\\Wrapper\\Availability",
      "type": "->"
    },
    {
      "file": "lib/private/Files/View.php",
      "line": 1342,
      "function": "file_exists",
      "class": "OC\\Files\\Storage\\Wrapper\\Wrapper",
      "type": "->"
    },
    {
      "file": "lib/private/Files/View.php",
      "line": 1383,
      "function": "getCacheEntry",
      "class": "OC\\Files\\View",
      "type": "->"
    },
    {
      "file": "lib/private/Files/Node/Root.php",
      "line": 208,
      "function": "getFileInfo",
      "class": "OC\\Files\\View",
      "type": "->"
    },
    {
      "file": "lib/private/Files/Node/Folder.php",
      "line": 139,
      "function": "get",
      "class": "OC\\Files\\Node\\Root",
      "type": "->"
    },
    {
      "file": "lib/private/Files/Node/Folder.php",
      "line": 148,
      "function": "get",
      "class": "OC\\Files\\Node\\Folder",
      "type": "->"
    },
    {
      "file": "apps/text/lib/Service/WorkspaceService.php",
      "line": 27,
      "function": "nodeExists",
      "class": "OC\\Files\\Node\\Folder",
      "type": "->"
    },
    {
      "file": "apps/text/lib/DAV/WorkspacePlugin.php",
      "line": 119,
      "function": "getFile",
      "class": "OCA\\Text\\Service\\WorkspaceService",
      "type": "->"
    },
    {
      "file": "3rdparty/sabre/event/lib/WildcardEmitterTrait.php",
      "line": 89,
      "function": "propFind",
      "class": "OCA\\Text\\DAV\\WorkspacePlugin",
      "type": "->"
    },
    {
      "file": "3rdparty/sabre/dav/lib/DAV/Server.php",
      "line": 1052,
      "function": "emit",
      "class": "Sabre\\DAV\\Server",
      "type": "->"
    },
    {
      "file": "3rdparty/sabre/dav/lib/DAV/Server.php",
      "line": 984,
      "function": "getPropertiesByNode",
      "class": "Sabre\\DAV\\Server",
      "type": "->"
    },
    {
      "file": "3rdparty/sabre/dav/lib/DAV/Server.php",
      "line": 1662,
      "function": "getPropertiesIteratorForPath",
      "class": "Sabre\\DAV\\Server",
      "type": "->"
    },
    {
      "file": "3rdparty/sabre/dav/lib/DAV/Server.php",
      "line": 1647,
      "function": "writeMultiStatus",
      "class": "Sabre\\DAV\\Server",
      "type": "->"
    },
    {
      "file": "3rdparty/sabre/dav/lib/DAV/CorePlugin.php",
      "line": 346,
      "function": "generateMultiStatus",
      "class": "Sabre\\DAV\\Server",
      "type": "->"
    },
    {
      "file": "3rdparty/sabre/event/lib/WildcardEmitterTrait.php",
      "line": 89,
      "function": "httpPropFind",
      "class": "Sabre\\DAV\\CorePlugin",
      "type": "->"
    },
    {
      "file": "3rdparty/sabre/dav/lib/DAV/Server.php",
      "line": 472,
      "function": "emit",
      "class": "Sabre\\DAV\\Server",
      "type": "->"
    },
    {
      "file": "3rdparty/sabre/dav/lib/DAV/Server.php",
      "line": 253,
      "function": "invokeMethod",
      "class": "Sabre\\DAV\\Server",
      "type": "->"
    },
    {
      "file": "3rdparty/sabre/dav/lib/DAV/Server.php",
      "line": 321,
      "function": "start",
      "class": "Sabre\\DAV\\Server",
      "type": "->"
    },
    {
      "file": "apps/dav/lib/Server.php",
      "line": 383,
      "function": "exec",
      "class": "Sabre\\DAV\\Server",
      "type": "->"
    },
    {
      "file": "apps/dav/appinfo/v2/remote.php",
      "line": 35,
      "function": "exec",
      "class": "OCA\\DAV\\Server",
      "type": "->"
    },
    {
      "file": "remote.php",
      "line": 172,
      "args": [
        "apps/dav/appinfo/v2/remote.php"
      ],
      "function": "require_once"
    }
  ],
  "File": "lib/private/Files/Storage/DAV.php",
  "Line": 889,
  "Hint": "Storage is temporarily not available",
  "message": "External storage not available: Sabre\\HTTP\\ClientHttpException: Internal Server Error",
  "exception": {},
  "CustomMessage": "External storage not available: Sabre\\HTTP\\ClientHttpException: Internal Server Error"
}

[mejo-]

/backport to stable29

[backportbot]

The backport to stable29 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable29
git pull origin stable29

# Create the new backport branch
git checkout -b backport/5959/stable29

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 5020d9b0

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/5959/stable29

Error: Failed to push branch backport/5959/stable29: fatal: could not read Username for 'https://github.com': No such device or address

---

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

[mejo-]

What do you think about a backport?

Agreed. Manual backport as backportbot failed for some reason: #6461