Skip to content

Commit d4f5ce9

Browse files
RENERENE
authored andcommitted
[MINOR] Fix flag parsing bug in FileInterpreter
Fixed a bug where the dash character (-) was incorrectly included as a flag when parsing command arguments. Now only the actual flag characters after the dash are added to the flags set. Added unit tests to verify the correct parsing behavior.
1 parent 1fe6b04 commit d4f5ce9

File tree

2 files changed

+182
-1
lines changed

2 files changed

+182
-1
lines changed

file/src/main/java/org/apache/zeppelin/file/FileInterpreter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public CommandArgs(String cmd) {
7070

7171
private void parseArg(String arg) {
7272
if (arg.charAt(0) == '-') { // handle flags
73-
for (int i = 0; i < arg.length(); i++) {
73+
for (int i = 1; i < arg.length(); i++) {
7474
Character c = arg.charAt(i);
7575
flags.add(c);
7676
}
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package org.apache.zeppelin.file;
20+
21+
import static org.junit.jupiter.api.Assertions.assertEquals;
22+
import static org.junit.jupiter.api.Assertions.assertTrue;
23+
import static org.junit.jupiter.api.Assertions.assertFalse;
24+
25+
import java.util.Properties;
26+
27+
import org.apache.zeppelin.interpreter.InterpreterException;
28+
import org.junit.jupiter.api.Test;
29+
30+
/**
31+
* Tests for FileInterpreter CommandArgs parsing functionality.
32+
*/
33+
class FileInterpreterTest {
34+
35+
/**
36+
* Mock FileInterpreter for testing CommandArgs functionality
37+
*/
38+
private static class TestFileInterpreter extends FileInterpreter {
39+
public TestFileInterpreter(Properties property) {
40+
super(property);
41+
}
42+
43+
@Override
44+
public String listAll(String path) throws InterpreterException {
45+
return "";
46+
}
47+
48+
@Override
49+
public boolean isDirectory(String path) {
50+
return true;
51+
}
52+
53+
@Override
54+
public void open() {
55+
}
56+
57+
@Override
58+
public void close() {
59+
}
60+
61+
// Expose CommandArgs for testing
62+
public CommandArgs getCommandArgs(String cmd) {
63+
CommandArgs args = new CommandArgs(cmd);
64+
args.parseArgs();
65+
return args;
66+
}
67+
}
68+
69+
@Test
70+
void testCommandArgsParsing() {
71+
TestFileInterpreter interpreter = new TestFileInterpreter(new Properties());
72+
73+
// Test simple command without flags
74+
FileInterpreter.CommandArgs args1 = interpreter.getCommandArgs("ls");
75+
assertEquals("ls", args1.command);
76+
assertEquals(0, args1.args.size());
77+
assertEquals(0, args1.flags.size());
78+
79+
// Test command with path
80+
FileInterpreter.CommandArgs args2 = interpreter.getCommandArgs("ls /user");
81+
assertEquals("ls", args2.command);
82+
assertEquals(1, args2.args.size());
83+
assertEquals("/user", args2.args.get(0));
84+
assertEquals(0, args2.flags.size());
85+
86+
// Test command with single flag
87+
FileInterpreter.CommandArgs args3 = interpreter.getCommandArgs("ls -l");
88+
assertEquals("ls", args3.command);
89+
assertEquals(0, args3.args.size());
90+
assertEquals(1, args3.flags.size());
91+
assertTrue(args3.flags.contains('l'));
92+
assertFalse(args3.flags.contains('-'));
93+
94+
// Test command with multiple flags
95+
FileInterpreter.CommandArgs args4 = interpreter.getCommandArgs("ls -la");
96+
assertEquals("ls", args4.command);
97+
assertEquals(0, args4.args.size());
98+
assertEquals(2, args4.flags.size());
99+
assertTrue(args4.flags.contains('l'));
100+
assertTrue(args4.flags.contains('a'));
101+
assertFalse(args4.flags.contains('-'));
102+
103+
// Test command with flags and path
104+
FileInterpreter.CommandArgs args5 = interpreter.getCommandArgs("ls -l /user");
105+
assertEquals("ls", args5.command);
106+
assertEquals(1, args5.args.size());
107+
assertEquals("/user", args5.args.get(0));
108+
assertEquals(1, args5.flags.size());
109+
assertTrue(args5.flags.contains('l'));
110+
assertFalse(args5.flags.contains('-'));
111+
112+
// Test command with separate flags
113+
FileInterpreter.CommandArgs args6 = interpreter.getCommandArgs("ls -l -h /user");
114+
assertEquals("ls", args6.command);
115+
assertEquals(1, args6.args.size());
116+
assertEquals("/user", args6.args.get(0));
117+
assertEquals(2, args6.flags.size());
118+
assertTrue(args6.flags.contains('l'));
119+
assertTrue(args6.flags.contains('h'));
120+
assertFalse(args6.flags.contains('-'));
121+
122+
// Test command with combined flags
123+
FileInterpreter.CommandArgs args7 = interpreter.getCommandArgs("ls -lah /user");
124+
assertEquals("ls", args7.command);
125+
assertEquals(1, args7.args.size());
126+
assertEquals("/user", args7.args.get(0));
127+
assertEquals(3, args7.flags.size());
128+
assertTrue(args7.flags.contains('l'));
129+
assertTrue(args7.flags.contains('a'));
130+
assertTrue(args7.flags.contains('h'));
131+
assertFalse(args7.flags.contains('-'));
132+
}
133+
134+
@Test
135+
void testCommandArgsWithDashNotInFlags() {
136+
TestFileInterpreter interpreter = new TestFileInterpreter(new Properties());
137+
138+
// Test that dash character is not included in flags after fix
139+
FileInterpreter.CommandArgs args = interpreter.getCommandArgs("ls -l");
140+
141+
// Verify dash is not in flags
142+
assertFalse(args.flags.contains('-'),
143+
"Dash character should not be included in flags");
144+
145+
// Verify correct flag is included
146+
assertTrue(args.flags.contains('l'),
147+
"Flag 'l' should be included");
148+
149+
// Verify flag count
150+
assertEquals(1, args.flags.size(),
151+
"Should only have one flag character");
152+
}
153+
154+
@Test
155+
void testEmptyFlags() {
156+
TestFileInterpreter interpreter = new TestFileInterpreter(new Properties());
157+
158+
// Test empty flag (just dash)
159+
FileInterpreter.CommandArgs args = interpreter.getCommandArgs("ls -");
160+
assertEquals("ls", args.command);
161+
assertEquals(0, args.args.size());
162+
assertEquals(0, args.flags.size());
163+
}
164+
165+
@Test
166+
void testComplexCommand() {
167+
TestFileInterpreter interpreter = new TestFileInterpreter(new Properties());
168+
169+
// Test complex command with multiple flags and arguments
170+
FileInterpreter.CommandArgs args = interpreter.getCommandArgs("ls -la -h /user /tmp");
171+
assertEquals("ls", args.command);
172+
assertEquals(2, args.args.size());
173+
assertEquals("/user", args.args.get(0));
174+
assertEquals("/tmp", args.args.get(1));
175+
assertEquals(3, args.flags.size());
176+
assertTrue(args.flags.contains('l'));
177+
assertTrue(args.flags.contains('a'));
178+
assertTrue(args.flags.contains('h'));
179+
assertFalse(args.flags.contains('-'));
180+
}
181+
}

0 commit comments

Comments
 (0)