Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix collectives on current Nextcloud main branch #1535

Closed
max-nextcloud opened this issue Oct 14, 2024 · 4 comments
Closed

Fix collectives on current Nextcloud main branch #1535

max-nextcloud opened this issue Oct 14, 2024 · 4 comments

Comments

@max-nextcloud
Copy link
Collaborator

There's been a change in the signature of IStorage:
nextcloud/server@f28e74b#diff-f7bada9f6beaa1f713c302710e2b73283f5df5c7100966ba7e9a08b4cb2acef9

This leads to requests failing with:

{
  "reqId": "6D48tHHIvcdaAElQ9yq3",
  "level": 3,
  "time": "2024-10-14T02:04:35+00:00",
  "remoteAddr": "127.0.0.1",
  "user": "bob",
  "app": "PHP",
  "method": "POST",
  "url": "/index.php/login",
  "message": "Declaration of OCA\\Collectives\\ACL\\ACLStorageWrapper::stat($path) must be compatible with OC\\Files\\Storage\\Wrapper\\Wrapper::stat(string $path): array|false at /home/runner/work/collectives/collectives/apps/collectives/lib/ACL/ACLStorageWrapper.php#167",
  "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Cypress/13.6.4 Chrome/114.0.5735.289 Electron/25.8.4 Safari/537.36",
  "version": "31.0.0.3",
  "data": {
    "app": "PHP"
  }
}
@max-nextcloud
Copy link
Collaborator Author

I attempted to fix this in #1534 - but that fails psalm with:

ERROR: MethodSignatureMismatch - lib/ACL/ACLStorageWrapper.php:167:30
 - Argument 1 of OCA\Collectives\ACL\ACLStorageWrapper::stat has wrong type 'string',
 expecting '' as defined by OCP\Files\Storage\IStorage::stat (see https://psalm.dev/042)
	public function stat(string $path): array|false {

@max-nextcloud
Copy link
Collaborator Author

@provokateurin How would i best handle this?
(ideally while keeping collectives compatibility with Nextcloud 28 to 30)

@provokateurin
Copy link
Member

Adding the return type is backwards compatible.
To keep backwards compatibility with older version you don't add the param types now, but once Nextcloud 31 is the lowest supported version. You are not required to add them to be in line with the interface, as allowing more types by not specifying the type is allowed.

@juliusknorr
Copy link
Member

Duplicate of #1518

@juliusknorr juliusknorr marked this as a duplicate of #1518 Oct 14, 2024
@juliusknorr juliusknorr closed this as not planned Won't fix, can't repro, duplicate, stale Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants