LDEV-6297 manifest maven: field works end-to-end on 6.2 - #2781
Open
zspitzer wants to merge 3 commits into
Open
Conversation
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.
Summary
maven:field accepted only JSON form; the canonical gradle GAV-comma form (perMAVEN-EXTENSION-MIGRATION.md, accepted by 7.x) failed silently with a JSON syntax error logged twice per install..lexwith a gradle GAV-comma manifest and havecreateObject(\"java\", className)resolve against the registered artefacts on 6.2 + 7.x without a server restart.JIRA: LDEV-6297
The three commits
b7875ce— accept gradle GAV-comma form in manifestmaven:fieldRHExtension.toMavenSettings(Log, String)helper. Tries JSON first (preserves existing 6.2 behaviour byte-for-byte), falls back toMavenUtil.toGAVSOs(str, null)for the gradlegroup:artifact:version[,...]form, converts each GAVSO back into the existingMap<String,String>shape soConfigAdmin._updateMavenand downstream consumers stay byte-identical.MavenUtil.toGAVSOsalready shipped on 6.2 under LDEV-5276 — this just wires it into the manifest read path.test/tickets/LDEV6297.cfccovers 8 format permutations via reflection (single GAV, multi-GAV, JSON struct, JSON array, short-key aliases, whitespace-padded, scope token, and a negative case).Syntax Error in JSON) with a single descriptive WARN-level log only when both formats fail.2463aa0— refresh cached JavaSettings after extension install_updateMaveninunSyncUpdateExtensionwrites maven entries toroot.javasettings, callsConfigWebFactory._loadJavaSettings(null, (ConfigImpl) config, root, logger)to rebuild the cachedJavaSettingsonConfigServerImpl. Reuses the existing boot-time helper (visibility widened fromprivateto package-private; same package)._updateMaven's entries persisted to.CFConfig.jsonand waited for the next server restart. With it, the same boot-time wiring fires post-install.6cb3e1c— also invalidate the JVM-level default classloader cacheModernApplicationContext.defaultClassLoaderis aprivate static ClassLoadercached at first use. Both Modern and Classic application contexts route throughModernApplicationContext.getDefaultClassLoader(ConfigWeb)for the request-time classloader. Once cached against pre-install (empty)JavaSettings, refreshingConfigServer.javaSettingshad no effect — the static cache was shadowing it.ModernApplicationContext.resetDefaultClassLoader()static method nulls the cache; called from the same hook as stage 2 immediately after the JavaSettings refresh.Why this doesn't mirror 7.x's pattern
7.x took a different architectural path:
ConfigImpl.resetJavaSettings()exists and is invoked viaResetFilterreflection (filter.add(\"resetJavaSettings\")inConfigAdmin.unSyncUpdateExtension).ModernApplicationContext.getRPCClassLoader()is deprecated:throw new RuntimeException(\"the method [getRPCClassLoader()] is no longer supported\");.getDefaultClassLoader(ConfigWeb)exists on 7.x but is uncalled anywhere in the codebase — dead code that survived the deprecation.createObject(\"java\", ...)path bypasses the static cache entirely.Backporting 7.x's path (
ResetFilter,resetAll(filter)reflection,getRPCClassLoaderdeprecation,JavaSettingsImpl.merge, ApplicationContext semantic changes) would be ~300-500 LoC of stable-branch infrastructure work touching application context resolution. We chose the surgical 6.2-specific fix that addresses the actual cache-staleness bug 6.2 has.The new
resetDefaultClassLoader()helper is intentionally branch-specific. 7.x doesn't need an equivalent because its classloader path doesn't read the cache. Future merges from 6.2 → 7.x should drop this helper.Empirical validation
Probe: 4
createObject(\"java\", className)calls against POI classes (HSSFWorkbook,XSSFWorkbook,SXSSFWorkbook,StreamingReader) usinglucee-spreadsheet-extension'sfeat/maven-refactor.lexwith a gradle-form manifest.Confirmed working with
lucee-spreadsheet-extensionend-to-end (cfspreadsheet operations resolve POI classes correctly post-install).Limitations on 6.2 vs 7.x (intentionally out of scope)
Application.cfcthis.javaSettingsmerges with server-level. On 6.2 it still replaces. Apps that don't overridethis.javaSettingssee server-level entries fine.Missing required field: version. Inverse of the original 6.2 bug. Not fixed here because the canonical form (per migration guide) is gradle GAV-comma — and after this PR lands, that form works on every supported Lucee branch.Test plan
test/tickets/LDEV6297.cfcpass (parser format coverage)mvn testclean for affected paths (LDEV6297 8/8; pre-existing flakesLDEV0407/LDEV4955are environmental — HTTP test target and VSCode env-var leak respectively)feat/maven-refactor.lexinstalls and resolves POI viacreateObjecton hot install with no restart.lexresolves POI on post-restart (regression check)