Code Quality, SonarQube and Best Practices#

#28 – Code Quality, SonarQube and Best Practices

Target: 3–4 years experienced AEM Developer Covers from your additional list: the Cloud Manager code quality gate in depth, SonarQube and OakPAL rules that matter in practice, and coding standards / review habits. Project domain: a global energy technology company's marketing site.

#Before we start — the gap between "it works" and "it passes the gate"

Files 26 and 27 both ended by pointing here, and for the same reason: a pipeline that builds successfully and a pipeline that clears the code quality gate are two different achievements. You can write a Sling Model that does exactly what the ticket asked, test it thoroughly, and still watch a Cloud Manager pipeline stop dead at Code Quality because of something the ticket never mentioned — an unclosed ResourceResolver, a cognitive-complexity score on a method nobody thought was complicated, or a security hotspot on a line that's actually fine but was never explicitly marked reviewed.

The honest framing for an interview: SonarQube and OakPAL aren't obstacles bolted onto AEM development — they're automating a review a senior developer would have done by hand anyway, at a speed and consistency no human review cadence can match. Knowing what they actually check, and why those specific things, is what turns "the pipeline failed" from a mystery into a five-minute fix.

#1. Introduction

#1.1 Where this sits relative to file 26

File 26 established the shape: Code Quality is the second gate in the pipeline, it runs SonarQube plus OakPAL, and it can block a deployment before anything reaches Stage. This file is what's actually inside that gate — the categories of finding that matter for AEM/Java code specifically, and the habits that keep you from meeting them for the first time in a failed pipeline run.

#1.2 Two tools, two different questions

• SonarQube asks: "Is this code well-written?" — bugs, vulnerabilities, code smells, test coverage, duplication.
• OakPAL asks: "Will this package do the right thing to the repository?" — filter correctness, forbidden paths, package structure.

Both show up as "Code Quality gate failed," and confusing which one flagged a problem wastes real debugging time — a theme file 26 already introduced and this file makes concrete with actual rule categories.

#1.3 A real project example

Requirement. A new servlet needed to query products by category for an internal tool.

What made it a code-quality story, not just a code story. The first version worked correctly in every manual test. The pipeline blocked it anyway — a security hotspot on a line building a JCR-SQL2 query by concatenating a request parameter directly into the query string, and a code smell on the servlet's doGet method, which had grown to a cognitive complexity score well past the threshold after several conditional branches were added over two sprints.

The fix. The query was rewritten to bind the parameter safely rather than concatenate it (file 21's territory, but a Sonar rule catches it independently of whether it happens to be exploitable in this specific case — the rule doesn't know that, and shouldn't have to). The method was split into three smaller ones, each doing one part of the validation.

The lesson. Neither finding was wrong to raise, even though neither was actively causing a production bug that day. That's the point of the gate — catching a pattern before it becomes an incident, not after.

#2. Core Concepts

#2.1 What SonarQube actually measures

Five categories, and knowing the names cold matters because interviewers ask for them directly:

• Bugs — code that's likely to behave incorrectly (a null pointer risk, a resource that's opened but not always closed on every path).
• Vulnerabilities — a demonstrated security weakness (e.g., a query built by string concatenation from user input).
• Security Hotspots — code that needs a human to look at it to decide if it's actually a vulnerability in context — not an automatic fail, but a required, explicit review.
• Code Smells — maintainability problems that aren't bugs yet: high cognitive complexity, duplicated logic, an oversized class, a method with too many parameters.
• Coverage & Duplication — percentage of new code exercised by tests, and percentage of code that's a near-duplicate of code elsewhere.

#2.2 "New Code," not "all code" — the single most important concept in this file

Adobe's Cloud Manager quality gate is built around Clean as You Code: thresholds apply to new or changed code in this pull request, not a retroactive score for the entire codebase. This is why a ten-year-old AEM 6.5-to-Cloud-Service migration with plenty of historical debt can still ship through Cloud Manager cleanly — the gate isn't asking "is this whole codebase perfect," it's asking "did you make it worse."

Why this matters practically: it means you cannot blame a blocked pipeline on "legacy code that was already like that." If the gate is failing, the finding is almost always on a line your diff touched. It also means refactoring a large old method you didn't otherwise need to touch can backfire — touching it pulls its pre-existing debt into the "new code" window and can surface old smells as if they were new.

#2.3 Quality Profile vs. Quality Gate — two different Sonar concepts people conflate

• A Quality Profile is the set of rules being applied — Adobe ships an AEM-specific profile with rules tuned for Sling, OSGi, and JCR patterns, on top of the general Java profile.
• A Quality Gate is the pass/fail threshold applied to a scan's results — e.g., zero new blocker bugs, all new security hotspots reviewed, coverage on new code above a set percentage.

You can have a rule fire (profile) without it failing the build (gate), if it's below the severity the gate cares about. Interviewers who ask "what's the difference between a profile and a gate" are checking whether you've actually read a Sonar report or just seen the word "SonarQube" in a pipeline log.

#2.4 The AEM-specific findings that come up again and again

Building on the pattern file 14 already named ("deprecated API use, admin sessions, package structure"), the recurring, AEM-specific findings worth recognising on sight:

• Unclosed ResourceResolver — acquired via resourceResolverFactory.getServiceResourceResolver(...) and never closed on every code path, including exception paths. This is the JCR session leak from file 20, caught statically before it ever exhausts a session pool in production.
• Use of getAdministrativeResourceResolver() or equivalent admin-session shortcuts — deprecated and flagged, because it bypasses the service-user permission model file 13 covers, and because it doesn't work at all in some Cloud Service contexts.
• Deprecated Sling/AEM API usage — a method or class marked @Deprecated still compiles and runs, but Sonar flags the call so it doesn't quietly survive into a version where it's removed entirely.
• System.out.println or raw exception printing instead of a proper SLF4J logger call — a code smell, because it bypasses log level control and doesn't show up in the log aggregation Cloud Manager relies on.
• High cognitive complexity on doGet / doPost methods and Sling Model @PostConstruct methods specifically, because those are exactly the methods that accumulate one more if every sprint until nobody can trace the logic.
• Hardcoded configuration values (a URL, a path, a credential-shaped string) instead of an OSGi @ObjectClassDefinition property (file 06) — flagged as both a smell and, for anything credential-shaped, a security hotspot.

#2.5 What OakPAL adds on top, restated with more rule detail

File 26 introduced OakPAL as "structural package validation." The categories worth naming specifically:

• Filter validity — every path a package actually writes to must be covered by a declared filter root; OakPAL can catch content that would install outside the declared scope.
• Forbidden paths — writes to paths a project has explicitly disallowed (a common project-level rule: nothing should ever be packaged under /home, echoing file 25's data-protection point).
• Import mode correctness — flags a ui.content filter without mode="merge" where the project's own OakPAL configuration requires it, catching file 25's "deployment that deleted a week of content" story before the package is ever installed anywhere.
• Node type sanity — content using node types the project doesn't expect, often a sign of copy-pasted content from a different environment or a botched export/import.

#2.6 Review habits beyond the automated gate

Tooling catches patterns; it doesn't catch judgment. A senior reviewer still asks questions no rule engine asks: does this Sling Model handle a half-filled multifield the way file 05 warned about? does this new service user actually need write access, or was jcr:all granted out of convenience? The gate is a floor, not a substitute for a human reviewing the change with the same production-mindedness this whole repository has been building toward.

#3. Internal Working

#3.1 How the "new code" delta is actually computed

Sonar compares the branch being analysed against a baseline — typically the last analysis of the branch it will merge into. Lines that differ from that baseline are "new code"; everything else is "overall code," measured but not gate-enforced. This is why the gate result can change without you changing your code — if the baseline branch moves (someone else merges), your diff's boundary against it can shift too.

#3.2 Local analysis versus the pipeline's analysis

A sonar-maven-plugin (or a standalone sonar-scanner) run locally, pointed at the same Sonar server and using the same quality profile, produces the same findings the pipeline would — which is exactly why file 26 recommended running it before opening a pull request. The pipeline's run is authoritative for the gate decision; a local run is a rehearsal, not a substitute.

#3.3 Marking a finding reviewed versus fixing it

A bug or code smell is resolved by changing the code — there's no "acknowledge and move on" option that clears the gate. A security hotspot is different by design: because it needs human judgment about context, Sonar requires it to be explicitly marked Reviewed (safe, with a reason) or Fixed, in the tool itself, before the gate treats it as resolved. Silently ignoring a hotspot leaves the gate blocked; there's no bypass at the pipeline level (consistent with file 26 — no gate has a bypass).

#3.4 Where coverage numbers actually come from

The coverage percentage reported to Sonar comes from a test-coverage tool (typically JaCoCo) run during the Maven build — the same unit tests file 27 covers. Coverage is computed only for new code, meaning a change that adds 40 untested lines to an existing well-tested class can still fail the gate, even though the class's overall coverage number looks fine.

#8. Coding Examples — common findings and their fixes

#8.1 Unclosed ResourceResolver → try-with-resources

// FLAGGED: resolver is never closed if getProducts() throws
public List<Product> loadProducts() throws LoginException {
    ResourceResolver resolver = resolverFactory.getServiceResourceResolver(AUTH_INFO);
    return getProducts(resolver); // if this throws, resolver leaks forever
}

// FIXED: try-with-resources guarantees close() on every exit path,
// including an exception — this is the file 20 leak, closed at the source.
public List<Product> loadProducts() throws LoginException {
    try (ResourceResolver resolver = resolverFactory.getServiceResourceResolver(AUTH_INFO)) {
        return getProducts(resolver);
    }
}

#8.2 Admin session shortcut → service user

// FLAGGED: bypasses the service-user permission model entirely (file 13),
// and doesn't work the same way in every Cloud Service context.
ResourceResolver resolver = resolverFactory.getAdministrativeResourceResolver(null);

// FIXED: a named service user with exactly the permissions this code needs.
Map<String, Object> authInfo = Collections.singletonMap(
    ResourceResolverFactory.SUBSERVICE, "product-catalog-service");
ResourceResolver resolver = resolverFactory.getServiceResourceResolver(authInfo);

#8.3 High cognitive complexity → extracted methods

// FLAGGED: one method doing validation, lookup, and formatting,
// with nested conditionals that accumulated over several sprints.
public String renderPrice(Resource resource) {
    if (resource != null) {
        ValueMap vm = resource.getValueMap();
        if (vm.get("price", Double.class) != null) {
            double price = vm.get("price", Double.class);
            if (price > 0) {
                if (vm.get("currency", String.class) != null) {
                    // ... more nesting to format currency-specific output
                }
            }
        }
    }
    return "";
}

// FIXED: each responsibility split into its own small, testable method.
// Cognitive complexity drops because each method reads as one clear idea,
// and each one can be unit tested in isolation (file 27's territory).
public String renderPrice(Resource resource) {
    if (resource == null) return "";
    Double price = resource.getValueMap().get("price", Double.class);
    if (price == null || price <= 0) return "";
    String currency = resource.getValueMap().get("currency", String.class);
    return formatCurrency(price, currency);
}

private String formatCurrency(double price, String currency) {
    // ... formatting logic alone, easy to reason about and test
    return String.format("%.2f %s", price, currency != null ? currency : "USD");
}

#8.4 Hardcoded configuration → OSGi property

// FLAGGED as both a smell and a hotspot if it looks credential-shaped.
private static final String API_ENDPOINT = "https://internal-api.example.com/v2/products";

// FIXED: an OSGi configuration property (file 06), so the value
// is environment-specific and never hardcoded into compiled code.
@ObjectClassDefinition(name = "Product Catalog Service Config")
@interface Config {
    @AttributeDefinition(name = "API Endpoint")
    String apiEndpoint() default "";
}

#9. Common Mistakes

#10. Best Practices

• Run Sonar locally before opening a pull request, using the same profile the pipeline enforces — catching a finding on your own machine costs minutes; catching it in the pipeline costs a full run.
• Treat every security hotspot as requiring an actual decision, documented with a reason when marked reviewed — never a reflexive dismissal.
• Keep refactoring separate from feature work where possible, so old debt surfacing as "new code" doesn't block an unrelated feature.
• Always close a ResourceResolver (or JCR Session) with try-with-resources, on every code path, without exception.
• Never use an admin-session shortcut — use a named service user scoped to exactly what the code needs (file 13).
• Push configuration, not hardcoded values, into OSGi properties — anything environment-specific or credential-shaped belongs there, not in compiled code.
• Write OakPAL rules in response to real incidents, the way the mode="merge" check in Story 3 was — a quality gate configuration should get stricter exactly where the project has actually been burned before.
• Don't treat a green pipeline as equivalent to "reviewed." The gate catches mechanical patterns; a human still needs to judge architectural fit and business-logic correctness.

#11. Debugging Tips

#12. Performance Notes

• Running Sonar analysis locally adds build time, but it's minutes against the alternative of a full pipeline round trip that can take much longer end to end (file 26).
• A large, uninterrupted refactor pull request analyses slower and risks pulling more old debt into the new-code window than several smaller, focused ones.
• Coverage instrumentation (JaCoCo) adds modest overhead to the test run itself — negligible next to the cost of discovering a coverage gap only in the pipeline.

#15. Comparison Tables

#Next topic

Security best practices for AEM developers — the security-specific patterns and mistakes this file touched on structurally (unclosed sessions, admin shortcuts, query injection) covered in depth, alongside AEM-specific concerns like XSS through HTL, CSRF protection, and dispatcher-level security already introduced in file 19.

Topic 28 of the AEM Interview Preparation repository. Teaching style, energy-sector project domain.