Skip to content

fix(QTDI-3531): Fix uispec goal failure - #1286

Open
undx wants to merge 6 commits into
masterfrom
undx/QTDI-3531_fix_uispec_goal_failure
Open

undx wants to merge 6 commits into
masterfrom
undx/QTDI-3531_fix_uispec_goal_failure

Conversation

@undx

@undx undx commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Requirements

  • Any code change adding any logic MUST be tested through a unit test executed with the default build
  • Any API addition MUST be done with a documentation update if relevant

Why this PR is needed?

Jira: QTDI-3531 — talend-component:uispec fails, for two independent reasons:

  1. VerifyError when run on a connectors repo. The plugin classloader realm contains the plugin dependencies plus the transitive dependencies of every connector CAR (844 jars in connectors). It includes cxf-rt-transports-http:4.1.8 (jakarta servlet) ahead of 3.6.12 (javax servlet) that component-server needs, so ServiceListGeneratorServlet is a jakarta servlet where a javax one is expected.
  2. NullPointerException in StaticResourceGenerator.route(...). The /api/v1/cache/clear route (QTDI-1188) is built with null content, so content.getBytes(...) fails even with a clean classpath. Nothing tested uispec, so it went unnoticed.

What does this PR adds (design/code thoughts)?

  • StaticResourceGenerator.route(...) is null-safe (null content → empty body) + unit test StaticResourceGeneratorTest.
  • UiSpecGeneratorMojo runs StaticUiSpecGenerator in an isolated URLClassLoader (parent = platform loader, TCCL set and restored, loader closed). Its classpath is the Aether-resolved graph of the plugin's own component-tools-webapp + component-runtime-beam + openwebbeans-se, so it no longer depends on CAR transitives in the plugin realm. Versions come from a filtered uispec-generator.properties.
  • New invoker IT src/it/uispec: builds a CAR dependency plus a conflicting cxf-rt-transports-http:4.1.8 plugin dependency, runs prepare-repository + uispec, and asserts uispec.zip contains component/ and configuration/ entries.
  • talend-component-maven-plugin/pom.xml: resource filtering for the properties file and openwebbeans-se declaration.

Validation: unit tests, RAT/checkstyle/spotless and invoker IT uispec green; talend-component:uispec validated manually in connectors/cloud/cloud-components-docker (-PPUSH_DOCKER, local SNAPSHOT) producing uispec.zip.

Out of scope: changing connectors' POMs, the web goal, other mojos' classloader strategy.

AI contribution metrics

  • Code Generation % (this PR): 100% (309/309 changed lines)
  • Code Generation % (ticket-wide cumulative): 100%
  • Technical Design % (ticket-wide cumulative): 100%

AI generated code

https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code

  • this PR has been written with the help of GitHub Copilot or another generative AI tool

Run the static UI spec generator in an isolated classloader built from
the plugin's own component-tools-webapp graph, so transitive
dependencies of connector CARs (e.g. CXF 4 / jakarta servlet) no longer
shadow the ones component-server needs and trigger a VerifyError.

Make StaticResourceGenerator.route null-safe so the
/api/v1/cache/clear route no longer fails with a NullPointerException.

Add a unit test and an invoker IT guarding the uispec goal.

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@undx

undx commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

Scope & Design Review

Critic Round 1 — Findings

# Severity File Description Suggested Fix
1 MINOR talend-component-maven-plugin/pom.xml component-tools-webapp is no longer imported by the Mojo but stays a plugin dependency; kept on purpose so the reactor builds/installs it first and the isolated resolution finds the SNAPSHOT. None (documented here).
2 MINOR UiSpecGeneratorMojo Isolated classpath is resolved with the plugin version for component-tools-webapp/component-runtime-beam — assumes lock-step versions (true for this reactor/releases). None.
3 MINOR StaticResourceGeneratorTest Uses JUnit assertions (module convention) rather than AssertJ. Optional.

Checked: null-safety of route, TCCL restored in finally, loader closed (try-with-resources), generator exceptions propagate unchanged, version resource lookup (package-relative) and Maven filtering, offline/SNAPSHOT resolution through the build session, threadSafe mojo (per-invocation loader, per-thread TCCL), IT reproduces the realm pollution (CXF 4.1.8 plugin dependency) and asserts zip content.

Approval gate: APPROVED — no Blocker or Major findings. 3 Minor noted (non-blocking).

Signed: Claude Sonnet 5.5

@undx

undx commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

Compliance Report

Files reviewed (7): StaticResourceGenerator.java, StaticResourceGeneratorTest.java, UiSpecGeneratorMojo.java, talend-component-maven-plugin/pom.xml, uispec-generator.properties, src/it/uispec/pom.xml, src/it/uispec/postbuild.groovy.

File Severity Finding Rule
StaticResourceGeneratorTest.java ℹ️ Info JUnit assertions instead of AssertJ (matches module convention) Testing
talend-component-maven-plugin/pom.xml ℹ️ Info Resources filtering only for the properties file; no new dependency/CVE surface (libs resolved at runtime from existing managed versions) Dependencies
Working tree ℹ️ Info Untracked AGENTS.md, CLAUDE.md, CONTEXT-MAP.md, docs/agents/* are unrelated to QTDI-3531 — must NOT be included in the PR Scope discipline

No wildcard imports, try-with-resources used, specific exceptions with cause preserved, Apache headers present, Spotless/RAT/checkstyle green (step 5).

Critical fixed: 0, Warnings: 0, Info: 3

Signed: Claude Sonnet 5.5

undx and others added 3 commits October 5, 2026 16:45
…tion knowledge

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…erver

Maven 3.10+ rejects duplicate <plugin> declarations for the same
groupId/artifactId at POM validation, breaking `mvn -pl <module> -am`
and whole-reactor builds that include component-starter-server. The
plugin was split into two <plugin> blocks (one carrying
<dependencies>, the other <executions>) with identical
<configuration>; merge them into a single declaration.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The uispec invoker IT's .mvn/jvm.config only opened java.net, java.lang
and java.util, missing java.lang.invoke (and others). On JDK 17+ this
makes the embedded Meecrowave/Tomcat + CDI server fail at startup with
InaccessibleObjectException on MethodHandles$Lookup, failing the IT
both locally and on Jenkins CI. Complete the set to match the
--add-opens list already used by the root pom's java9 profile.

Note: .mvn/ directories are not tracked anywhere else in this repo and
are commonly excluded by global gitignore templates; this file was
force-added since the invoker plugin requires it to configure the
forked build's JVM.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Critical Maven wrapper and dependency-resolution issues remain, and new UiSpec logic lacks default-build unit coverage.

Review effort: Lite
Findings: 2 High severity · 1 Medium severity · 1 Low severity

Open (4)
What changed in this PR

Fixes talend:uispec classpath conflicts and null-content generation failures.

Changes:

  • Isolates UiSpec generator dependencies with a dedicated classloader.
  • Makes static routes null-safe with unit coverage.
  • Adds integration coverage, filtered dependencies, and documentation updates.
File Summary
talend-component-maven-plugin/​src/​main/​resources/​org/​talend/​sdk/​component/​maven/​uispec-generator.properties Defines filtered generator dependency versions.
talend-component-maven-plugin/​src/​main/​java/​org/​talend/​sdk/​component/​maven/​UiSpecGeneratorMojo.java Implements isolated generator loading and dependency resolution.
talend-component-maven-plugin/​src/​it/​uispec/​postbuild.groovy Validates generated UiSpec archive contents.
talend-component-maven-plugin/​src/​it/​uispec/​pom.xml Adds conflicting dependency integration-test setup.
talend-component-maven-plugin/​src/​it/​uispec/​.mvn/​jvm.config Configures required JVM module access.
talend-component-maven-plugin/​pom.xml Configures resource filtering and generator dependencies.
repository-knowledge.md Documents UiSpec verification guidance.
component-tools-webapp/​src/​test/​java/​org/​talend/​sdk/​component/​tools/​webapp/​standalone/​generator/​StaticResourceGeneratorTest.java Tests null and UTF-8 route content.
component-tools-webapp/​src/​main/​java/​org/​talend/​sdk/​component/​tools/​webapp/​standalone/​generator/​StaticResourceGenerator.java Handles null route content safely.
component-starter-server/​pom.xml Alters Maven wrapper generation configuration; execution binding requires correction.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread component-starter-server/pom.xml
Comment thread repository-knowledge.md Outdated
Resolve the generator classpath from the project repositories (the
webapp, Beam and OpenWebBeans are regular dependencies, not plugins),
extract the isolated run into a package-private method and add default
build unit tests for the classloader isolation, TCCL restoration and
error wrapping. Document the -Pci-build invoker command.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@undx

undx commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

Review round 1 — summary

Comment Class Action
#1 — gmavenplus execution detached (component-starter-server/pom.xml) Already addressed Replied, see 1860795f4cf3
#2 — resolver uses plugin repositories Code fix Fixed in 0bf42b04ef1d
#3 — no default-build unit test for classloader logic Code fix Fixed in 0bf42b04ef1d (UiSpecGeneratorMojoTest, 6 tests pass)
#4 — IT command omits -Pci-build Code fix (doc) Fixed in 0bf42b04ef1d
Sonar — coverage on new code < 80% Code fix (covered by #3) New unit tests pushed; Aether resolution in isolatedClasspath() stays covered by the invoker IT only. Gate result to be checked after CI.

Fixes pushed: 0bf42b04ef1d — fix(QTDI-3531): Address review round 1 on uispec mojo
Pending clarifications: 0
Rebase: none (base not moved)

Round summary generated by AI. Please resolve threads after verifying the fixes.

Signed: GitHub Copilot

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The isolated classloader omits required runtime dependencies, potentially causing NoClassDefFoundError during CDI startup.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (4)

Extract the classpath resolution into a package-private static method
and cover it with unit tests using a stubbed RepositorySystem. Hoist
the arguments out of the assertThrows lambdas (java:S5778).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@undx

undx commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

Review round 2 — summary

Comment Class Action
Copilot — UiSpecGeneratorMojo.java "isolated graph omits component-runtime-manager / component-runtime-design-extension" Already addressed (false positive) Replied: both are transitive compile deps of component-tools-webapp (via component-server); the uispec invoker IT passes
Sonar java:S5778 — UiSpecGeneratorMojoTest assertThrows lambdas Code fix Fixed in 52215f2 — arguments hoisted out of the 4 lambdas, assertions unchanged
Sonar Quality Gate — coverage on new code (44%) Code fix Fixed in 52215f2 — classpath resolution extracted into package-private resolveClasspath(...) (no behaviour change) and covered by 3 new tests with a stubbed RepositorySystem (roots/scope/repositories, URL conversion, failure wrapping)

Fixes pushed: 52215f2 — fix(QTDI-3531): Address review round 2 on uispec mojo
Pending clarifications: 0
Rebase: not needed (origin/master not ahead)

Round summary generated by AI. Please resolve threads after verifying the fixes. The Sonar gate result will be known once CI re-analyses this push.

Signed: Claude Opus 4.8

@sonar-rnd

sonar-rnd Bot commented Oct 5, 2026

Copy link
Copy Markdown

Failed Quality Gate failed

  • 78.00% Coverage on New Code (is less than 80.00%)
  • 1 New Issues (is greater than 0)

Project ID: org.talend.sdk.component:component-runtime

View in SonarQube

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

All reviewed changes are covered by tests, and no unresolved blocking issues were identified.

Review effort: Lite
Findings: None

Resolved since last review (1)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants