Skip to content

lower log level for quota manipulation cases - #10034

Merged
blizzz merged 1 commit into
masterfrom
fix/noid/ldap-silence-quota-logmsgs
Jun 28, 2018
Merged

lower log level for quota manipulation cases#10034
blizzz merged 1 commit into
masterfrom
fix/noid/ldap-silence-quota-logmsgs

Conversation

@blizzz

Copy link
Copy Markdown
Member

that's all relevant for debug, and if nothing is set it is not even worth to write anything at all. I also decreased the nesting of the ifs.

$quota = $aQuota[0];
} else {
$this->log->log('not suitable LDAP quota found for user ' . $this->uid . ': [' . $valueFromLDAP . ']', ILogger::WARN);
$this->log->log('no suitable LDAP quota found for user ' . $this->uid . ': [' . $aQuota[0] . ']', ILogger::DEBUG);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The $aQuota[0] could cause issues, because this else branch is also triggered when $aQuota determines to false or the count is 0 or lower.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

In that case, readAttribute would need to return an array, of size > 0, that does not use numeric indexes. This is not the case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hu? I'm not following this.

readAttribute can return false.
In that case $aQuota would be false. The check fails. And we try to read $aQuota[0] which does 💥

Or am I missing something?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

🤔 ok

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

doesn't log anything now when nothing was found, as was before.

and simplify the forest of ifs a little bit
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
blizzzforce-pushed the fix/noid/ldap-silence-quota-logmsgs branch from 1489a32 to cc51a00CompareJune 28, 2018 19:10
@codecov

codecovBot commented Jun 28, 2018

Copy link
Copy Markdown

Codecov Report

Merging #10034 into master will increase coverage by 20.27%.
The diff coverage is 95.23%.

@@ Coverage Diff @@## master #10034 +/- ##
=============================================
+ Coverage 31.72% 51.99% +20.27% - Complexity 26007 26011 +4 
=============================================
Files 1661 1661 Lines 96132 96130 -2 Branches 1290 1290 =============================================
+ Hits 30494 49986 +19492 + Misses 65638 46144 -19494
Impacted FilesCoverage ΔComplexity Δ
apps/user_ldap/lib/Connection.php55.18% <ø> (+55.18%)121 <0> (ø)⬇️
apps/user_ldap/lib/User/User.php76.09% <95.23%> (+76.09%)127 <0> (+4)⬆️
lib/private/Files/Stream/Encryption.php75.22% <0%> (+0.45%)54% <0%> (ø)⬇️
apps/theming/lib/ThemingDefaults.php97.29% <0%> (+0.67%)52% <0%> (ø)⬇️
lib/private/Group/Group.php76.19% <0%> (+0.79%)62% <0%> (ø)⬇️
lib/private/Setup.php12.55% <0%> (+0.83%)52% <0%> (ø)⬇️
lib/public/AppFramework/Db/Mapper.php94.95% <0%> (+0.84%)30% <0%> (ø)⬇️
...vate/Authentication/Token/DefaultTokenProvider.php98.05% <0%> (+0.97%)32% <0%> (ø)⬇️
lib/private/Preview/Generator.php78.28% <0%> (+1.14%)53% <0%> (ø)⬇️
lib/private/App/DependencyAnalyzer.php95.62% <0%> (+1.25%)64% <0%> (ø)⬇️
... and 382 more

@blizzz
blizzz merged commit 28e64af into masterJun 28, 2018
@blizzz
blizzz deleted the fix/noid/ldap-silence-quota-logmsgs branch June 28, 2018 21:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsbugfeature: ldap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@blizzz@rullzer@MorrisJobke