Enable SMB logging for the Community Edition

[mmattel]

Description

New config.php parameter smb.logging.enable to enable SMB logging.
A parameter for this already exists for the Enterprise edition wnd.logging.enable , but to seperate this properly, a community parameter is needed. The code for this already existed, it was just about to add the new parameter additionally and document it.

Related Issue

Motivation and Context

Discussion with @PVince81

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

owncloud-archive/documentation#3715

[PVince81]

I'm fine with this if you guys are too @jvillafanez @butonic

[codecov]

Codecov Report

Merging #30185 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master   #30185      +/-   ##
============================================
- Coverage     58.24%   58.23%   -0.01%     
  Complexity    18548    18548              
============================================
  Files          1092     1092              
  Lines         63729    63729              
============================================
- Hits          37116    37115       -1     
- Misses        26613    26614       +1

Impacted Files	Coverage Δ	Complexity Δ
drone/src/apps/files_trashbin/lib/Expiration.php	96.55% <0%> (-1.73%)	29% <0%> (ø)
drone/src/lib/private/Server.php	82.55% <0%> (ø)	129% <0%> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6b9286...5aec95e. Read the comment docs.

[jvillafanez]

I'd just rename the configuration option: less changes => less problems. Having both seems useless to me.
That option is merely for debugging, so there shouldn't be anyone actively using it, specially due to its heavy verbosity. Changing the name doesn't have any impact.

[phil-davis]

s/a an existing/an existing
and this text describing the returns could go in the return PHPdoc line below

[mmattel]

oh, will correct this. Thought that a return without an preceeding @ is ok to phpdoc not to parse it as a keyword.

[mmattel]

@jvillafanez wnd is exclusively used by the Enterprise Edition (see https://marketplace.owncloud.com/apps/windows_network_drive) and it is already documented in documentation/admin_manual/configuration/server/config_sample_php_parameters.rst (This enables debug logs for the windows_network_drive app.)

The intention was to not mix up Enterprise and Community parameters and keep this clearly seperated...

[jvillafanez]

That part of the code is already complex enough to add even more complexity, that's why if the easiest solution (renaming the variable) works, then let's go with it.

Renaming the variable doesn't have any impact in any of both apps (SMB and WND).

The fact that "wnd" is there is because of a slip when this was added here, and kind of laziness to not change it afterwards (me included). There is no need to keep it if we're going to change it. If WND wanted to add special logs or anything, it has to do it on its own app anyway.

We'll have to update the documentation accordingly, but other than that I don't think we need any degree of compatibility here: that switch isn't intended to be actively used so it shouldn't matter if we change the switch.

[PVince81]

Ok, I agree with @jvillafanez.

This setting should anyway not be used in production as it would slow down and fill log files very quickly, so I don't expect anyone to have this. Just in case, we can still mention it in the next release notes.

[mmattel]

Just to have the tasks clear:

• reject the current code changes in this PR
• rename the parameter wnd.logging.enable to smb.logging.enable and wnd to smb
• keep the config.sample.php documentation part
• change in an seperate PR the current documentation (see above) to reflect this change

Let me know if this is correct or if I missed something.

[jvillafanez]

Yes, that's it

[mmattel]

If this gets merged, I will do a backport to stable10 if wanted.

[mmattel]

Ready to merge?

[mmattel]

@PVince81
Shall I backport?

[PVince81]

@mmattel yes, please go ahead. Thanks

[phil-davis]

Backport stable10 #30244

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.