[
https://issues.apache.org/jira/browse/MNG-8462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Tamas Cservenak updated MNG-8462:
---------------------------------
Description:
Current stance is way better what MavenCli was, but still is quite dirty (and
problematic):
* context is "naked", is not encapsulated and is really just a mixed bag of
exposed data fields and some methods
* invoker reentrancy is problematic, as it is coupled to value (null/non-null)
of context "naked" fields, causes issues like MNG-8461 was
It should be refactored:
* if you consider, we have "stages" executing
* "stage methods" should be aware are they "first time" or "re entering" (or
they could be just "do always")
* context should be encapsulated
* context should be aware of stage when fields are set (so they could "unset" a
stage?)
* context should be "forked" (in scenario similar to MNG-8461) when context is
to be reused but should not affect "main stages", this could also simplify
reentrancy/residency, see "copier" in BuiltinShellCommandRegistryFactory, there
"forking" happens (of shell context)
Current stages could be (are defined already somewhat):
{noformat}
protected int doInvoke(C context) throws Exception {
pushCoreProperties(context);
pushUserProperties(context);
configureLogging(context);
createTerminal(context);
activateLogging(context);
helpOrVersionAndMayExit(context);
preCommands(context);
container(context);
postContainer(context);
pushUserProperties(context); // after PropertyContributor SPI
lookup(context);
init(context);
postCommands(context);
settings(context);
return execute(context);
}
{noformat}
Basically "stage methods" could just be tracking "past stages" and depending on
state (first time? re entering stage?) could invoke/perform proper code
(instead to tie it to context field value).
To me this is akin to some simple "state machine" with enums as stages (see
method calls above, but distinguish pushUserProperties 1/2), and then even
debug log could contain stages with first/re-entering nfo, basically improving
debug capability, but also lowering problems like the originating bug
introduced (as you explicitly code "first time" and "re entering" cases, not
mixing)
was:
Current stance is way better what MavenCli was, but still is quite dirty (and
problematic):
* context is "naked", is not encapsulated and is really just a mixed bag of
exposed data fields and some methods
* invoker reentrancy is problematic, as it is coupled to value (null/non-null)
of context "naked" fields, causes issues like MNG-8461 was
It should be refactored:
* if you consider, we have "stages" executing
* "stage methods" should be aware are they "first time" or "re entering" (or
they could be just "do always")
* context should be encapsulated
* context should be aware of stage when fields are set (so they could "unset" a
stage?)
* context should be "forked" (in scenario similar to MNG-8461) when context is
to be reused but should not affect "main stages", this could also simplify
reentrancy/residency, see "copier" in BuiltinShellCommandRegistryFactory, there
"forking" happens (of shell context)
Current stages could be (are defined already somewhat):
{noformat}
protected int doInvoke(C context) throws Exception {
pushCoreProperties(context);
pushUserProperties(context);
configureLogging(context);
createTerminal(context);
activateLogging(context);
helpOrVersionAndMayExit(context);
preCommands(context);
container(context);
postContainer(context);
pushUserProperties(context); // after PropertyContributor SPI
lookup(context);
init(context);
postCommands(context);
settings(context);
return execute(context);
}
{noformat}
Basically "stage methods" could just be tracking "past stages" and depending on
state (first time? re entering stage?) could invoke/perform proper code
(instead to tie it to context field value).
> Refactor maven-cli:Invoker and/or Context
> -----------------------------------------
>
> Key: MNG-8462
> URL: https://issues.apache.org/jira/browse/MNG-8462
> Project: Maven
> Issue Type: Task
> Components: Command Line
> Reporter: Tamas Cservenak
> Priority: Major
>
> Current stance is way better what MavenCli was, but still is quite dirty (and
> problematic):
> * context is "naked", is not encapsulated and is really just a mixed bag of
> exposed data fields and some methods
> * invoker reentrancy is problematic, as it is coupled to value
> (null/non-null) of context "naked" fields, causes issues like MNG-8461 was
> It should be refactored:
> * if you consider, we have "stages" executing
> * "stage methods" should be aware are they "first time" or "re entering" (or
> they could be just "do always")
> * context should be encapsulated
> * context should be aware of stage when fields are set (so they could "unset"
> a stage?)
> * context should be "forked" (in scenario similar to MNG-8461) when context
> is to be reused but should not affect "main stages", this could also simplify
> reentrancy/residency, see "copier" in BuiltinShellCommandRegistryFactory,
> there "forking" happens (of shell context)
> Current stages could be (are defined already somewhat):
> {noformat}
> protected int doInvoke(C context) throws Exception {
> pushCoreProperties(context);
> pushUserProperties(context);
> configureLogging(context);
> createTerminal(context);
> activateLogging(context);
> helpOrVersionAndMayExit(context);
> preCommands(context);
> container(context);
> postContainer(context);
> pushUserProperties(context); // after PropertyContributor SPI
> lookup(context);
> init(context);
> postCommands(context);
> settings(context);
> return execute(context);
> }
> {noformat}
> Basically "stage methods" could just be tracking "past stages" and depending
> on state (first time? re entering stage?) could invoke/perform proper code
> (instead to tie it to context field value).
> To me this is akin to some simple "state machine" with enums as stages (see
> method calls above, but distinguish pushUserProperties 1/2), and then even
> debug log could contain stages with first/re-entering nfo, basically
> improving debug capability, but also lowering problems like the originating
> bug introduced (as you explicitly code "first time" and "re entering" cases,
> not mixing)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
