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

files: Local#writeStream should use it's own file_put_contents #24594

Merged
merged 2 commits into from
Dec 22, 2020

Conversation

kofemann
Copy link
Member

@kofemann kofemann commented Dec 7, 2020

The OC\Files\Storage\Local#writeStream use system provided file_put_contents.
However, it overrides file_put_contents, thus expects that the default behaviour
can be different.

Use Local#file_put_contents in writeStream to benefit from class specific functionality.

Signed-off-by: Tigran Mkrtchyan tigran.mkrtchyan@desy.de

The OC\Files\Storage\Local#writeStream use system provided file_put_contents.
However, it overrides file_put_contents, thus expects that the default behaviour
can be different.

Use Local#file_put_contents in writeStream to benefit from class specific functionality.

Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
@kesselb kesselb added 3. to review Waiting for reviews enhancement labels Dec 7, 2020
@kesselb kesselb requested a review from rullzer December 7, 2020 18:07
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Yes makes sense.

@rullzer rullzer added this to the Nextcloud 21 milestone Dec 15, 2020
This was referenced Dec 16, 2020
@juliushaertl
Copy link
Member

Psalm is not happy about that. Actually the annotation of file_put_contents in

* @param string $data
should be a resource instead of a string and we'd need to cast the return value of writeStream to be an integer.

@kofemann
Copy link
Member Author

Ok. I can fix that.

@kofemann
Copy link
Member Author

As in other places it used as systems put_file_contents, I will docs as return int|false

@juliushaertl
Copy link
Member

As in other places it used as systems put_file_contents, I will docs as return int|false

Yeah, that would indeed be the correct one 👍

@kofemann kofemann force-pushed the dcache branch 6 times, most recently from 2fe6090 to c7aabba Compare December 21, 2020 17:57
The current phpdoc of IStorage#file_put_contents doesnt corresponds to
it's actual usage in code, e.g.

Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
@juliushaertl
Copy link
Member

Thanks a lot 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants