[
https://issues.apache.org/jira/browse/WW-5692?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart updated WW-5692:
------------------------------
Fix Version/s: 8.0.0
(was: 7.4.0)
Description:
Part of the WW-5689 lean-core work. Retargeted from 7.4.0 to 8.0.0 and
re-scoped from an in-place rewrite to a module move.
{{DebuggingInterceptor}} holds an injected {{FreemarkerManager}} and constructs
{{FreemarkerResult}} directly to render {{console.ftl}} and {{browser.ftl}}. It
is the third of the three places core reaches into {{views.freemarker}}, after
WW-5690/WW-5693 and WW-5691.
h2. Why a move rather than a rewrite
The sibling tickets replace their template with a plain-Java renderer. That is
the wrong trade here:
* {{browser.ftl}} is 91 lines and {{console.ftl}} 34 — substantially more than
{{wait.ftl}}, and {{console.ftl}} uses the tag library too ({{s.url
value="/static"}}), not just FreeMarker.
* A rewrite inherits the escaping trap described in WW-5691 and WW-5693:
{{FreemarkerManager}} sets {{HTMLOutputFormat}} with
{{ENABLE_IF_DEFAULT_AUTO_ESCAPING_POLICY}}, so these templates are HTML-escaped
today by configuration. Hand-written renderers lose that, and this one renders
the entire value stack.
* Moving the interceptor takes its templates with it. No rewrite, no escaping
change, and existing template overrides keep working.
This is also the better fit for WW-5689's direction: a devMode-only diagnostic
tool is a plugin, not core.
h2. The package is already clean
{{org.apache.struts2.interceptor.debugging}} contains exactly three classes —
{{DebuggingInterceptor}}, {{ObjectToHTMLWriter}}, {{PrettyPrintWriter}} — and
the two writers are referenced nowhere else in core, the plugins or the apps.
So the whole-package cut that WW-4759 requires is available here with no
de-mixing work first, unlike most of WW-5689.
What it needs from core is ordinary published API: {{ActionContext}},
{{ActionInvocation}}, {{ServletActionContext}}, {{AbstractInterceptor}},
{{ValueStack}}, {{ReflectionProvider}}, {{ThreadAllowlist}}, and some
{{dispatcher}} types. Nothing that would invert the dependency.
h2. The open design question: defaultStack membership
This is the part that must be settled before any code moves.
{{struts-default.xml}} declares the interceptor (line 57) and references it
from *{{defaultStack}}* (line 230). A plugin cannot simply take that with it —
core's {{defaultStack}} cannot reference an interceptor that may not be on the
classpath, and every application extends {{struts-default}}.
Candidate resolutions, none obviously correct:
# Core's {{defaultStack}} drops {{debugging}}; the plugin publishes its own
stack that applications opt into. Simple and honest, but devMode debugging
stops working out of the box — a real regression for a well-liked tool.
# The plugin redefines {{defaultStack}}. Fragile, and it collides with any
other plugin doing the same.
# Add a core extension point letting a plugin append an interceptor to a named
stack. Cleanest for users, but it is new framework machinery and deserves its
own ticket rather than riding along on this one.
Option 1 is the least surprising if the assembly keeps bundling the plugin by
default, since a stock distribution would still ship it — but that only shifts
the problem to anyone building a dependency list by hand.
h2. Notes
* {{struts.devMode}} gates the whole interceptor, so no production behaviour is
at stake either way.
* The new plugin depends on the FreeMarker plugin and on core's static content
serving ({{console.ftl}} points its {{baseUrl}} at {{/static}}).
* Sequencing against WW-5689's module split needs care: this plugin is a
consumer of {{struts2-freemarker-plugin}}, so it should land after that module
exists, or carry the dependency temporarily.
was:
Part of the WW-5689 lean-core work, but self-contained and shippable in 7.x.
See WW-5690 for the shared scope note — this concerns _core's own_ FreeMarker
dependency only; tag-library users still require FreeMarker via the default FTL
themes.
h2. Current behaviour
{{DebuggingInterceptor}} holds an injected {{FreemarkerManager}} and constructs
{{FreemarkerResult}} directly in two places to render its diagnostic pages:
* the {{console}} mode, rendering
{{/org/apache/struts2/interceptor/debugging/console.ftl}}
* the {{browser}} mode, rendering
{{/org/apache/struts2/interceptor/debugging/browser.ftl}}
This is the third and last hard {{views.freemarker}} import left in core
outside the view packages, after WW-5690 and WW-5691.
h2. Proposal
Same treatment as the other two: render the debug pages without instantiating a
{{FreemarkerResult}}, keeping the output identical.
Worth considering as an alternative: this interceptor is a devMode-only
diagnostic tool and is arguably a plugin in its own right. Moving it out of
core wholesale would resolve the dependency without rewriting the templates,
and fits the WW-5689 direction. That is the larger change though — for 7.x,
removing the hard dependency in place is the safer scope, and the decision can
be revisited for 8.0.0.
h2. Compatibility
Both templates are internal. {{struts.devMode}} gates the whole interceptor, so
no production path is affected.
Summary: Move DebuggingInterceptor into its own plugin (was: Remove
core's FreeMarker dependency from DebuggingInterceptor)
> Move DebuggingInterceptor into its own plugin
> ---------------------------------------------
>
> Key: WW-5692
> URL: https://issues.apache.org/jira/browse/WW-5692
> Project: Struts 2
> Issue Type: Task
> Components: Core
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 8.0.0
>
>
> Part of the WW-5689 lean-core work. Retargeted from 7.4.0 to 8.0.0 and
> re-scoped from an in-place rewrite to a module move.
> {{DebuggingInterceptor}} holds an injected {{FreemarkerManager}} and
> constructs {{FreemarkerResult}} directly to render {{console.ftl}} and
> {{browser.ftl}}. It is the third of the three places core reaches into
> {{views.freemarker}}, after WW-5690/WW-5693 and WW-5691.
> h2. Why a move rather than a rewrite
> The sibling tickets replace their template with a plain-Java renderer. That
> is the wrong trade here:
> * {{browser.ftl}} is 91 lines and {{console.ftl}} 34 — substantially more
> than {{wait.ftl}}, and {{console.ftl}} uses the tag library too ({{s.url
> value="/static"}}), not just FreeMarker.
> * A rewrite inherits the escaping trap described in WW-5691 and WW-5693:
> {{FreemarkerManager}} sets {{HTMLOutputFormat}} with
> {{ENABLE_IF_DEFAULT_AUTO_ESCAPING_POLICY}}, so these templates are
> HTML-escaped today by configuration. Hand-written renderers lose that, and
> this one renders the entire value stack.
> * Moving the interceptor takes its templates with it. No rewrite, no escaping
> change, and existing template overrides keep working.
> This is also the better fit for WW-5689's direction: a devMode-only
> diagnostic tool is a plugin, not core.
> h2. The package is already clean
> {{org.apache.struts2.interceptor.debugging}} contains exactly three classes —
> {{DebuggingInterceptor}}, {{ObjectToHTMLWriter}}, {{PrettyPrintWriter}} — and
> the two writers are referenced nowhere else in core, the plugins or the apps.
> So the whole-package cut that WW-4759 requires is available here with no
> de-mixing work first, unlike most of WW-5689.
> What it needs from core is ordinary published API: {{ActionContext}},
> {{ActionInvocation}}, {{ServletActionContext}}, {{AbstractInterceptor}},
> {{ValueStack}}, {{ReflectionProvider}}, {{ThreadAllowlist}}, and some
> {{dispatcher}} types. Nothing that would invert the dependency.
> h2. The open design question: defaultStack membership
> This is the part that must be settled before any code moves.
> {{struts-default.xml}} declares the interceptor (line 57) and references it
> from *{{defaultStack}}* (line 230). A plugin cannot simply take that with it
> — core's {{defaultStack}} cannot reference an interceptor that may not be on
> the classpath, and every application extends {{struts-default}}.
> Candidate resolutions, none obviously correct:
> # Core's {{defaultStack}} drops {{debugging}}; the plugin publishes its own
> stack that applications opt into. Simple and honest, but devMode debugging
> stops working out of the box — a real regression for a well-liked tool.
> # The plugin redefines {{defaultStack}}. Fragile, and it collides with any
> other plugin doing the same.
> # Add a core extension point letting a plugin append an interceptor to a
> named stack. Cleanest for users, but it is new framework machinery and
> deserves its own ticket rather than riding along on this one.
> Option 1 is the least surprising if the assembly keeps bundling the plugin by
> default, since a stock distribution would still ship it — but that only
> shifts the problem to anyone building a dependency list by hand.
> h2. Notes
> * {{struts.devMode}} gates the whole interceptor, so no production behaviour
> is at stake either way.
> * The new plugin depends on the FreeMarker plugin and on core's static
> content serving ({{console.ftl}} points its {{baseUrl}} at {{/static}}).
> * Sequencing against WW-5689's module split needs care: this plugin is a
> consumer of {{struts2-freemarker-plugin}}, so it should land after that
> module exists, or carry the dependency temporarily.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
