Skip to content

Conversation

@alperozturk96
Copy link
Contributor

@alperozturk96 alperozturk96 commented May 8, 2025

Instead of executing the PUT method for each parameter update using client.executeMethod(put) inside the loop, we can first build all the parameters in the loop and then execute the PUT method only once.

To observe this behavior, open the Android Files app, update any share, and check the log or place a debugger in the UpdateShareRemoteOperation class.

@nextcloud nextcloud deleted a comment from github-actions bot May 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2025

SpotBugs

CategoryBaseNew
Bad practice3535
Correctness3434
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness33
Performance88
Total161161

Copy link
Contributor

@ZetaTom ZetaTom left a comment

Choose a reason for hiding this comment

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

Please consider using com.nextcloud.common.JSONRequestBody. Other than this everything looks good and works.

@alperozturk96 alperozturk96 force-pushed the prevent-unnecessary-update-share-remote-operation branch from 5045717 to dcca584 Compare May 26, 2025 04:30
@github-actions
Copy link
Contributor

SpotBugs

CategoryBaseNew
Bad practice3535
Correctness3434
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness33
Performance88
Total161161

@github-actions
Copy link
Contributor

SpotBugs

CategoryBaseNew
Bad practice3535
Correctness3434
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness33
Performance88
Total161161

@alperozturk96 alperozturk96 force-pushed the prevent-unnecessary-update-share-remote-operation branch from 6d07229 to d2f0864 Compare May 27, 2025 10:35
@github-actions
Copy link
Contributor

SpotBugs

CategoryBaseNew
Bad practice3535
Correctness3434
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness33
Performance88
Total161161

@codecov
Copy link

codecov bot commented May 27, 2025

Codecov Report

Attention: Patch coverage is 72.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 43.07%. Comparing base (d862794) to head (37d4709).
Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
...b/resources/shares/UpdateShareRemoteOperation.java 72.00% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master    #1722   +/-   ##
=========================================
  Coverage     43.06%   43.07%           
  Complexity      988      988           
=========================================
  Files           235      235           
  Lines          8568     8562    -6     
  Branches       1123     1119    -4     
=========================================
- Hits           3690     3688    -2     
+ Misses         4364     4359    -5     
- Partials        514      515    +1     
Files with missing lines Coverage Δ
...b/resources/shares/UpdateShareRemoteOperation.java 59.42% <72.00%> (+2.08%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ZetaTom
ZetaTom previously approved these changes May 30, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2025

SpotBugs

CategoryBaseNew
Bad practice3535
Correctness3434
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness33
Performance88
Total161161

@alperozturk96 alperozturk96 force-pushed the prevent-unnecessary-update-share-remote-operation branch from 60d608f to d3ad338 Compare June 2, 2025 09:42
@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2025

SpotBugs

CategoryBaseNew
Bad practice3534
Correctness3434
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness33
Performance88
Total161160

@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2025

SpotBugs

CategoryBaseNew
Bad practice3534
Correctness3434
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness33
Performance88
Total161160

@tobiasKaminsky
Copy link
Member

https://docs.nextcloud.com/server/latest/developer_manual/client_apis/OCS/ocs-share-api.html#update-share

@skjnldsv @come-nc (since you are app owner of files_sharing)
above documentation says:

Only one of the update parameters can be specified at once.

But according to Alper's test multiple parameter work.
So, is this documentation outdated, or are some combinations not allowed?

@come-nc
Copy link

come-nc commented Jun 2, 2025

This note dates from before my time. I see nothing in the controller code indicating such a limitation 🤷

ZetaTom
ZetaTom previously approved these changes Jun 4, 2025
@alperozturk96 alperozturk96 force-pushed the prevent-unnecessary-update-share-remote-operation branch from b55a094 to 37d4709 Compare June 10, 2025 08:39
@github-actions
Copy link
Contributor

SpotBugs

CategoryBaseNew
Bad practice3534
Correctness3435
Dodgy code2626
Internationalization66
Malicious code vulnerability4949
Multithreaded correctness33
Performance88
Total161161

@alperozturk96 alperozturk96 requested a review from ZetaTom June 10, 2025 10:45
@alperozturk96 alperozturk96 merged commit d24e114 into master Jun 10, 2025
19 checks passed
@alperozturk96 alperozturk96 deleted the prevent-unnecessary-update-share-remote-operation branch June 10, 2025 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants