Clear the Sonar findings on the branding settings code - #2132
Open
Jakub-Bartal-01 wants to merge 1 commit into
Open
Clear the Sonar findings on the branding settings code#2132Jakub-Bartal-01 wants to merge 1 commit into
Jakub-Bartal-01 wants to merge 1 commit into
Conversation
- SVG_LENGTH made the whitespace runs around the optional unit possessive, so a long run of spaces no longer takes quadratic time (java:S8786). - The PNG chunk walk is a while loop, since its advance depends on the chunk length read inside the body and cannot sit in a for update clause (java:S127). - SvgSanitizer names the "style" literal, used for both the element and the attribute (java:S1192). - The Javadoc for the unknown-stored-theme test moved onto that test instead of dangling above an unrelated one (java:S8491). - The PNG fixture in everyRejectionNamesTheFieldItCameFrom is built before the assertThrows lambda, leaving one throwing call inside it (java:S5778).
There was a problem hiding this comment.
Pull request overview
Clears five SonarCloud findings in branding validation, sanitization, and tests without changing behavior.
Changes:
- Makes SVG length matching linear and clarifies PNG chunk iteration.
- Reuses a constant for SVG style handling.
- Improves test fixture isolation and Javadoc placement.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
BrandingLogoValidator.java |
Uses possessive regex quantifiers and a clearer PNG loop. |
SvgSanitizer.java |
Centralizes the "style" identifier. |
BrandingLogoValidatorTest.java |
Builds the PNG fixture outside assertThrows. |
BrandingSettingsITest.java |
Associates Javadoc with the correct test. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Clears the five SonarCloud findings that #2081 and #2082 left on
main. No behaviour changes.BrandingLogoValidatorSVG_LENGTHmatched\s*(unit)?\s*$, so with no unit present both whitespace runs could claim the same spaces and the engine backtracked over every split. The runs are now possessive, which makes the match linear; a 40 000-space input goes from quadratic to 1 ms. The accepted language is unchanged.offsetfrom inside the loop body, because the step is the chunk length read from that chunk. It is awhileloop now, so nothing assigns to aforcounter.SvgSanitizer"style"appeared three times, for the<style>element and thestyleattribute. It is a named constant.Tests
aBrandingUpdateWaitsForAConcurrentOneToRelease, which it does not describe. Moved ontoanUnknownStoredThemeIsIgnoredRatherThanFailingTheRead.everyRejectionNamesTheFieldItCameFrombuilt its PNG fixture inside theassertThrowslambda. The fixture is built first, so only the call under test can throw from inside the lambda.