[MNG-8462] Refactor maven-cli:Invoker and/or Context
- Dominant language
- Java
- Stars
- 5.3k
- Forks
- 3.1k
- Avg merge
- 20h 42m
- Merged PRs (30d)
- 297
Description
**[Tamas Cservenak](https://issues.apache.org/jira/secure/ViewProfile.jspa?name=cstamas)** opened **[MNG-8462](https://issues.apache.org/jira/browse/MNG-8462?redirect=false)** and commented
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):
```
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);
}
```
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)
---
**Issue Links:**
- [MNG-8461](https://issues.apache.org/jira/browse/MNG-8461) SettingsBuilderRequest not emitted
Contributor guide
Research direction
Start by reading maven-cli:Invoker and Context, especially the doInvoke stage sequence and the reentrancy behavior related to MNG-8461. Compare the context-forking approach in BuiltinShellCommandRegistryFactory, then define and validate explicit stage transitions, encapsulated context state, and correct first-entry versus re-entry behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- build-system, cli
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100