JinwooHwang opened a new pull request, #7958:
URL: https://github.com/apache/geode/pull/7958
## Description
This PR fixes a missing banner issue, a history file migration issues and a
critical bug where `gfsh` fails to start with a `NullPointerException` after
the Spring Shell 1.x to 3.x migration.
**Related JIRA:** GEODE-10523
## Problem
After migrating to Spring Shell 3.x, running `gfsh` results in:
```
Exception in thread "main" java.lang.NullPointerException: Cannot invoke
"org.jline.reader.LineReader.readLine(String)" because "this.lineReader" is null
at
org.apache.geode.management.internal.cli.shell.Gfsh.promptLoop(Gfsh.java:622)
at
org.apache.geode.management.internal.cli.shell.Gfsh.run(Gfsh.java:282)
at
org.apache.geode.management.internal.cli.shell.Gfsh.main(Gfsh.java:235)
```
Additionally, two other issues were discovered:
1. History file format incompatibility between JLine 2 and JLine 3
2. Missing banner output in non-headless mode
## Root Cause
Spring Shell 1.x automatically initialized the terminal and line reader
during startup. Spring Shell 3.x requires explicit initialization of these
components before use. The `Gfsh.run()` method was calling `promptLoop()`
without initializing the terminal or creating the console reader first.
## Solution
### 1. Terminal Initialization
Added `initializeTerminal()` method to explicitly create and initialize the
JLine 3 Terminal:
```java
private void initializeTerminal() throws IOException {
if (terminal == null) {
terminal = TerminalBuilder.builder()
.system(true)
.build();
}
}
```
Modified `run()` method to initialize terminal and create console reader
before entering prompt loop:
```java
public int run(String... args) throws IOException {
printBannerAndWelcome();
initializeTerminal();
createConsoleReader();
String historyFileName = getHistoryFileName();
// ...
return promptLoop();
}
```
### 2. History File Migration
Implemented automatic migration from JLine 2 format (with `//` comments) to
JLine 3 format (clean format):
**In `Gfsh.java`:**
```java
private void migrateHistoryFileIfNeeded(Path historyPath) throws IOException
{
if (!Files.exists(historyPath)) {
return;
}
List<String> lines = Files.readAllLines(historyPath);
if (!lines.isEmpty() && (lines.get(0).startsWith("//") ||
lines.get(0).startsWith("#"))) {
Path backupPath = Paths.get(historyPath.toString() + ".old");
Files.move(historyPath, backupPath, StandardCopyOption.REPLACE_EXISTING);
logger.info("Migrated old history file format. Old file backed up to: "
+ backupPath);
}
}
```
**In `GfshHistory.java`:**
```java
@Override
public void attach(LineReader reader) {
try {
super.attach(reader);
} catch (IllegalArgumentException e) {
if (e.getMessage() != null && e.getMessage().contains("Bad history file
syntax")) {
migrateOldHistoryFile();
super.attach(reader);
} else {
throw e;
}
}
}
```
### 3. Banner Display Fix
Modified `printAsInfo()` to always print to stdout in addition to logging:
```java
private void printAsInfo(String message) {
// Always print to stdout for banner visibility
println(message);
// Also log when not headless
if (!isHeadlessMode()) {
logger.info(message);
}
}
```
## Files Modified
-
`geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java`
- Added `initializeTerminal()` method
- Added `migrateHistoryFileIfNeeded()` method
- Modified `run()` method to initialize terminal before prompt loop
- Modified `printAsInfo()` to output to stdout
- Added imports: `java.nio.file.Files`, `java.nio.file.Path`
-
`geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/jline/GfshHistory.java`
- Overrode `attach()` method with exception handling
- Added `migrateOldHistoryFile()` method for format migration
## Testing
### Build Verification
```bash
./gradlew build
```
**Result:** BUILD SUCCESSFUL
**Verified:**
- Banner displays correctly
- No NullPointerException
- History file automatically migrated on first run (old file backed up to
`.gfsh_history.old`)
- Interactive prompt works correctly
### Example Output
```
_________________________ __
/ _____/ ______/ ______/ /____/ /
/ / __/ /___ /_____ / _____ /
/ /__/ / ____/ _____/ / / / /
/______/_/ /______/_/ /_/ 1.16.0-build.0
Monitor and Manage Apache Geode
gfsh>
```
## Impact
- **Severity:** Critical - gfsh completely non-functional without this fix
- **Scope:** All users attempting to run gfsh after Spring Shell 3.x
migration
- **Risk:** Low - Changes are isolated to initialization code and backward
compatible
## Checklist
- [x] Code changes implemented and tested
- [x] Build passes successfully
- [x] Functional testing completed
- [x] No breaking changes to existing functionality
- [x] History migration is automatic and transparent to users
- [x] Old history files are safely backed up
<!-- Thank you for submitting a contribution to Apache Geode. -->
<!-- In order to streamline review of your contribution we ask that you
ensure you've taken the following steps. -->
### For all changes, please confirm:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in
the commit message?
- [x] Has your PR been rebased against the latest commit within the target
branch (typically `develop`)?
- [x] Is your initial contribution a single, squashed commit?
- [x] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies
licensed in a way that is compatible for inclusion under [ASF
2.0](http://www.apache.org/legal/resolved.html#category-a)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
