Resolve Client.Java-032..036: shared subscription base, batch tokenizer
Client.Java-032 README CLI examples for stream-alarms and
acknowledge-alarm now use the correct picocli flags
(--filter-prefix and --reference); two regression
tests parse each documented invocation.
Client.Java-033 StreamAlarmsCommand publishes an
AtomicReference<MxGatewayAlarmFeedSubscription> and
mirrors MxEventStream's overflow branch: a failed
queue.offer cancels the subscription, queues an
IllegalStateException, then queues the END sentinel
— preserving the fail-fast contract.
Client.Java-034 BatchCommand routes through a new
MxGatewayCli.tokenizeBatchLine POSIX-style shell
tokenizer that respects double-quoted, single-quoted,
and backslash-escaped arguments.
Client.Java-035 Added streamAlarmsForwardsRequestAndStreamsAlarmFeedMessages
to MxGatewayClientSessionTests; asserts request shape,
message ordering, and cancellation propagation.
Client.Java-036 Extracted MxGatewayStreamSubscription<TRequest,TResponse>
abstract base; the four subscription classes
(MxGatewayEventSubscription, MxGatewayAlarmFeedSubscription,
MxGatewayActiveAlarmsSubscription, DeployEventSubscription)
collapse to ~10-line subclasses. A new contract test
runs identical lifecycle / cancellation assertions
across all four subclasses.
All resolved at 2026-05-24; gradle build + gradle test BUILD SUCCESSFUL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+138
-4
@@ -33,6 +33,7 @@ import java.util.Optional;
|
||||
import java.util.concurrent.ArrayBlockingQueue;
|
||||
import java.util.concurrent.BlockingQueue;
|
||||
import java.util.concurrent.Callable;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest;
|
||||
import mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot;
|
||||
@@ -119,7 +120,7 @@ public final class MxGatewayCli implements Callable<Integer> {
|
||||
return 0;
|
||||
}
|
||||
|
||||
private static CommandLine commandLine(MxGatewayCliClientFactory clientFactory) {
|
||||
static CommandLine commandLine(MxGatewayCliClientFactory clientFactory) {
|
||||
CommandLine commandLine = new CommandLine(new MxGatewayCli(clientFactory));
|
||||
commandLine.addSubcommand("version", new VersionCommand());
|
||||
commandLine.addSubcommand("open-session", new OpenSessionCommand(clientFactory));
|
||||
@@ -154,6 +155,120 @@ public final class MxGatewayCli implements Callable<Integer> {
|
||||
/** Sentinel queued by {@code stream-alarms} to mark a clean end of the alarm feed. */
|
||||
private static final Object ALARM_FEED_END = new Object();
|
||||
|
||||
/**
|
||||
* Tokenises a single batch-mode stdin line into the argv that the inner
|
||||
* {@link CommandLine} should execute. Honours single-quoted, double-quoted,
|
||||
* and backslash-escaped runs so values that contain spaces (e.g.
|
||||
* {@code --comment "needs verification"}) survive intact — the old
|
||||
* implementation used {@code split("\\s+")} which shredded any quoted
|
||||
* argument mid-string (Client.Java-034).
|
||||
*
|
||||
* <p>Rules (a small POSIX-like shell tokenizer; no variable expansion,
|
||||
* command substitution, globbing, or backtick handling):
|
||||
*
|
||||
* <ul>
|
||||
* <li>Outside quotes, runs of whitespace separate tokens.</li>
|
||||
* <li>{@code "..."} groups a sequence into one token; the surrounding
|
||||
* quotes are removed. Inside double quotes a backslash escapes
|
||||
* {@code \\}, {@code "}, and a literal newline; other characters
|
||||
* are taken literally (so {@code \n} is the two characters
|
||||
* backslash-n).</li>
|
||||
* <li>{@code '...'} groups a sequence into one token; the surrounding
|
||||
* quotes are removed. Inside single quotes nothing is escaped —
|
||||
* the run is literal until the matching single quote.</li>
|
||||
* <li>Outside quotes, backslash escapes the next character (including
|
||||
* whitespace, so {@code needs\ verification} is one token).</li>
|
||||
* <li>An unterminated quote or a trailing backslash throws
|
||||
* {@link IllegalArgumentException} so the batch loop surfaces it
|
||||
* as a JSON error instead of silently emitting wrong args.</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p>Empty input (or input that contains only whitespace) returns an
|
||||
* empty array so callers can skip the line.
|
||||
*/
|
||||
static String[] tokenizeBatchLine(String line) {
|
||||
List<String> tokens = new ArrayList<>();
|
||||
StringBuilder current = new StringBuilder();
|
||||
boolean inToken = false;
|
||||
// 0 = outside, 1 = inside single quotes, 2 = inside double quotes
|
||||
int quoteMode = 0;
|
||||
int length = line.length();
|
||||
for (int i = 0; i < length; i++) {
|
||||
char c = line.charAt(i);
|
||||
if (quoteMode == 1) {
|
||||
if (c == '\'') {
|
||||
quoteMode = 0;
|
||||
} else {
|
||||
current.append(c);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (quoteMode == 2) {
|
||||
if (c == '\\') {
|
||||
if (i + 1 >= length) {
|
||||
throw new IllegalArgumentException(
|
||||
"batch tokenizer: trailing backslash inside double-quoted string");
|
||||
}
|
||||
char next = line.charAt(i + 1);
|
||||
if (next == '\\' || next == '"' || next == '\n') {
|
||||
current.append(next);
|
||||
i++;
|
||||
} else {
|
||||
// POSIX rule: inside double quotes a backslash is
|
||||
// literal unless it precedes \, ", $, `, or newline.
|
||||
current.append(c);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (c == '"') {
|
||||
quoteMode = 0;
|
||||
continue;
|
||||
}
|
||||
current.append(c);
|
||||
continue;
|
||||
}
|
||||
// Outside any quotes.
|
||||
if (c == '\'') {
|
||||
quoteMode = 1;
|
||||
inToken = true;
|
||||
continue;
|
||||
}
|
||||
if (c == '"') {
|
||||
quoteMode = 2;
|
||||
inToken = true;
|
||||
continue;
|
||||
}
|
||||
if (c == '\\') {
|
||||
if (i + 1 >= length) {
|
||||
throw new IllegalArgumentException(
|
||||
"batch tokenizer: trailing backslash outside quotes");
|
||||
}
|
||||
current.append(line.charAt(i + 1));
|
||||
i++;
|
||||
inToken = true;
|
||||
continue;
|
||||
}
|
||||
if (Character.isWhitespace(c)) {
|
||||
if (inToken) {
|
||||
tokens.add(current.toString());
|
||||
current.setLength(0);
|
||||
inToken = false;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
current.append(c);
|
||||
inToken = true;
|
||||
}
|
||||
if (quoteMode != 0) {
|
||||
throw new IllegalArgumentException(
|
||||
"batch tokenizer: unterminated " + (quoteMode == 1 ? "single" : "double") + " quote");
|
||||
}
|
||||
if (inToken) {
|
||||
tokens.add(current.toString());
|
||||
}
|
||||
return tokens.toArray(new String[0]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads one CLI invocation per stdin line, executes each via a fresh
|
||||
* {@link CommandLine}, and writes {@value #BATCH_EOR} to stdout after
|
||||
@@ -183,8 +298,8 @@ public final class MxGatewayCli implements Callable<Integer> {
|
||||
if (line.isEmpty()) {
|
||||
break;
|
||||
}
|
||||
String[] args = line.trim().split("\\s+");
|
||||
if (args.length == 0 || (args.length == 1 && args[0].isEmpty())) {
|
||||
String[] args = tokenizeBatchLine(line);
|
||||
if (args.length == 0) {
|
||||
continue;
|
||||
}
|
||||
StringWriter cmdOut = new StringWriter();
|
||||
@@ -1079,11 +1194,29 @@ public final class MxGatewayCli implements Callable<Integer> {
|
||||
StreamAlarmsRequest request = StreamAlarmsRequest.newBuilder()
|
||||
.setAlarmFilterPrefix(filterPrefix)
|
||||
.build();
|
||||
// Client.Java-033 — fail-fast on overflow. A bare
|
||||
// queue.offer(value) silently drops messages past capacity,
|
||||
// which violates the JavaStyleGuide "do not drop events"
|
||||
// contract and lets the CLI exit 0 on a truncated feed.
|
||||
// Mirrors MxEventStream's overflow branch: detect a failed
|
||||
// offer, cancel the subscription, drain the buffer, then
|
||||
// queue an explicit overflow exception followed by the END
|
||||
// sentinel so the drain loop surfaces a non-zero exit.
|
||||
AtomicReference<MxGatewayAlarmFeedSubscription> subscriptionRef = new AtomicReference<>();
|
||||
MxGatewayAlarmFeedSubscription subscription =
|
||||
client.streamAlarms(request, new StreamObserver<>() {
|
||||
@Override
|
||||
public void onNext(AlarmFeedMessage value) {
|
||||
queue.offer(value);
|
||||
if (!queue.offer(value)) {
|
||||
MxGatewayAlarmFeedSubscription sub = subscriptionRef.get();
|
||||
if (sub != null) {
|
||||
sub.cancel();
|
||||
}
|
||||
queue.clear();
|
||||
queue.offer(new IllegalStateException(
|
||||
"stream-alarms queue overflowed (capacity 1024); consumer too slow"));
|
||||
queue.offer(ALARM_FEED_END);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -1096,6 +1229,7 @@ public final class MxGatewayCli implements Callable<Integer> {
|
||||
queue.offer(ALARM_FEED_END);
|
||||
}
|
||||
});
|
||||
subscriptionRef.set(subscription);
|
||||
try {
|
||||
int count = 0;
|
||||
while (true) {
|
||||
|
||||
+216
@@ -225,6 +225,89 @@ final class MxGatewayCliTests {
|
||||
assertTrue(run.errors().contains("--reference"), run.errors());
|
||||
}
|
||||
|
||||
@Test
|
||||
void readmeDocumentedStreamAlarmsExampleParsesCleanly() {
|
||||
// Client.Java-032 regression — the README's stream-alarms example
|
||||
// (clients/java/README.md:182) must round-trip through picocli's
|
||||
// parser without a parse error. Before the fix, the example used
|
||||
// a non-existent --session-id option and picocli failed at parse
|
||||
// time. This test pins the exact tokens documented in the README.
|
||||
String[] args = {
|
||||
"stream-alarms",
|
||||
"--endpoint",
|
||||
"localhost:5000",
|
||||
"--api-key-env",
|
||||
"MXGATEWAY_API_KEY",
|
||||
"--plaintext",
|
||||
"--filter-prefix",
|
||||
"Galaxy",
|
||||
"--limit",
|
||||
"1",
|
||||
"--json"
|
||||
};
|
||||
assertReadmeExampleParses(args);
|
||||
}
|
||||
|
||||
@Test
|
||||
void readmeDocumentedAcknowledgeAlarmExampleParsesCleanly() {
|
||||
// Client.Java-032 regression — the README's acknowledge-alarm
|
||||
// example (clients/java/README.md:183) must parse without error.
|
||||
// Before the fix it used --session-id (no such option) and
|
||||
// --alarm-reference (the real option is --reference), so picocli
|
||||
// rejected the invocation immediately.
|
||||
String[] args = {
|
||||
"acknowledge-alarm",
|
||||
"--endpoint",
|
||||
"localhost:5000",
|
||||
"--api-key-env",
|
||||
"MXGATEWAY_API_KEY",
|
||||
"--plaintext",
|
||||
"--reference",
|
||||
"\\Galaxy\\Area001.Pump001.PumpFault",
|
||||
"--json"
|
||||
};
|
||||
assertReadmeExampleParses(args);
|
||||
}
|
||||
|
||||
/**
|
||||
* Parses the given args through the production picocli {@link CommandLine}
|
||||
* and asserts no parser error, no unknown option, and no missing required
|
||||
* option. Does not execute the command body — only the option / subcommand
|
||||
* parser is exercised, so no network call is made.
|
||||
*/
|
||||
private static void assertReadmeExampleParses(String[] args) {
|
||||
picocli.CommandLine commandLine = MxGatewayCli.commandLine(new FakeClientFactory());
|
||||
try {
|
||||
commandLine.parseArgs(args);
|
||||
} catch (picocli.CommandLine.ParameterException ex) {
|
||||
throw new AssertionError(
|
||||
"documented README invocation failed picocli parse: "
|
||||
+ String.join(" ", args)
|
||||
+ " -> "
|
||||
+ ex.getMessage(),
|
||||
ex);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void streamAlarmsCommandFailsFastOnQueueOverflow() {
|
||||
// Client.Java-033 regression — the CLI's stream-alarms bounded queue
|
||||
// used queue.offer(value) which silently dropped messages past
|
||||
// capacity (1024). After the fix the CLI must surface the overflow
|
||||
// as a non-zero exit (mirroring MxEventStream's fail-fast contract).
|
||||
//
|
||||
// The OverflowingFakeClient floods the gRPC observer with 2000
|
||||
// messages synchronously, which exceeds the bounded 1024-element
|
||||
// queue. The fix detects the failed offer, cancels the subscription,
|
||||
// queues an overflow exception, and the drain loop surfaces it.
|
||||
OverflowingFakeClientFactory factory = new OverflowingFakeClientFactory();
|
||||
CliRun run = execute(factory, "stream-alarms", "--filter-prefix", "Flood");
|
||||
|
||||
assertFalse(run.exitCode() == 0,
|
||||
"expected non-zero exit when the alarm queue overflows; got exit=" + run.exitCode()
|
||||
+ " out=\n" + run.output() + "\nerr=\n" + run.errors());
|
||||
}
|
||||
|
||||
@Test
|
||||
void batchCommandExecutesVersionAndEmitsEorMarker() {
|
||||
CliRun run = executeBatch(new FakeClientFactory(), "version --json\n");
|
||||
@@ -235,6 +318,68 @@ final class MxGatewayCliTests {
|
||||
assertTrue(out.contains(MxGatewayCli.BATCH_EOR), out);
|
||||
}
|
||||
|
||||
@Test
|
||||
void batchCommandTokenisesDoubleQuotedArgumentWithEmbeddedSpaces() {
|
||||
// Client.Java-034 regression — a real shell-style tokenizer must not
|
||||
// shred `"needs verification"` into two arguments. Drives
|
||||
// acknowledge-alarm through batch and asserts the captured --comment
|
||||
// is the un-quoted string with the embedded space preserved.
|
||||
FakeClientFactory factory = new FakeClientFactory();
|
||||
String line = "acknowledge-alarm --reference Tank01.Level.HiHi --comment \"needs verification\" --operator op1\n";
|
||||
CliRun run = executeBatch(factory, line);
|
||||
|
||||
assertEquals(0, run.exitCode());
|
||||
assertEquals("needs verification", factory.client.lastAcknowledgeAlarmRequest.getComment());
|
||||
assertEquals("op1", factory.client.lastAcknowledgeAlarmRequest.getOperatorUser());
|
||||
assertEquals(
|
||||
"Tank01.Level.HiHi", factory.client.lastAcknowledgeAlarmRequest.getAlarmFullReference());
|
||||
}
|
||||
|
||||
@Test
|
||||
void batchCommandTokenisesSingleQuotedArgumentWithEmbeddedSpaces() {
|
||||
FakeClientFactory factory = new FakeClientFactory();
|
||||
String line =
|
||||
"acknowledge-alarm --reference Tank01.Level.HiHi --comment 'needs verification' --operator op1\n";
|
||||
CliRun run = executeBatch(factory, line);
|
||||
|
||||
assertEquals(0, run.exitCode());
|
||||
assertEquals("needs verification", factory.client.lastAcknowledgeAlarmRequest.getComment());
|
||||
}
|
||||
|
||||
@Test
|
||||
void batchCommandTokenisesBackslashEscapedSpaceOutsideQuotes() {
|
||||
FakeClientFactory factory = new FakeClientFactory();
|
||||
String line =
|
||||
"acknowledge-alarm --reference Tank01.Level.HiHi --comment needs\\ verification\n";
|
||||
CliRun run = executeBatch(factory, line);
|
||||
|
||||
assertEquals(0, run.exitCode());
|
||||
assertEquals("needs verification", factory.client.lastAcknowledgeAlarmRequest.getComment());
|
||||
}
|
||||
|
||||
@Test
|
||||
void batchCommandPreservesEmptyQuotedArgument() {
|
||||
FakeClientFactory factory = new FakeClientFactory();
|
||||
String line = "acknowledge-alarm --reference Tank01.Level.HiHi --comment \"\"\n";
|
||||
CliRun run = executeBatch(factory, line);
|
||||
|
||||
assertEquals(0, run.exitCode());
|
||||
assertEquals("", factory.client.lastAcknowledgeAlarmRequest.getComment());
|
||||
}
|
||||
|
||||
@Test
|
||||
void batchCommandSupportsBackslashEscapedQuoteInsideDoubleQuotes() {
|
||||
// `--comment "with \"inner\" quote"` should round-trip the inner
|
||||
// double-quote into the comment string.
|
||||
FakeClientFactory factory = new FakeClientFactory();
|
||||
String line =
|
||||
"acknowledge-alarm --reference Tank01.Level.HiHi --comment \"with \\\"inner\\\" quote\"\n";
|
||||
CliRun run = executeBatch(factory, line);
|
||||
|
||||
assertEquals(0, run.exitCode());
|
||||
assertEquals("with \"inner\" quote", factory.client.lastAcknowledgeAlarmRequest.getComment());
|
||||
}
|
||||
|
||||
@Test
|
||||
void batchCommandEmitsEorAfterFailedCommandAndContinues() {
|
||||
// An unknown subcommand causes a picocli parse error (non-zero exit).
|
||||
@@ -290,6 +435,77 @@ final class MxGatewayCliTests {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Factory whose fake client floods the {@code streamAlarms} observer with
|
||||
* 2000 messages synchronously, exceeding the CLI's bounded 1024-element
|
||||
* queue. Used by the Client.Java-033 fail-fast overflow regression.
|
||||
*/
|
||||
private static final class OverflowingFakeClientFactory implements MxGatewayCli.MxGatewayCliClientFactory {
|
||||
@Override
|
||||
public MxGatewayCli.MxGatewayCliClient connect(MxGatewayCli.CommonOptions options) {
|
||||
return new OverflowingFakeClient(options.spec.commandLine().getOut());
|
||||
}
|
||||
}
|
||||
|
||||
private static final class OverflowingFakeClient implements MxGatewayCli.MxGatewayCliClient {
|
||||
private final PrintWriter out;
|
||||
|
||||
OverflowingFakeClient(PrintWriter out) {
|
||||
this.out = out;
|
||||
}
|
||||
|
||||
@Override
|
||||
public PrintWriter out() {
|
||||
return out;
|
||||
}
|
||||
|
||||
@Override
|
||||
public OpenSessionReply openSession(OpenSessionRequest request) {
|
||||
return OpenSessionReply.newBuilder().setSessionId("flood-session").setProtocolStatus(ok()).build();
|
||||
}
|
||||
|
||||
@Override
|
||||
public CloseSessionReply closeSession(CloseSessionRequest request) {
|
||||
return CloseSessionReply.newBuilder()
|
||||
.setSessionId(request.getSessionId())
|
||||
.setFinalState(SessionState.SESSION_STATE_CLOSED)
|
||||
.setProtocolStatus(ok())
|
||||
.build();
|
||||
}
|
||||
|
||||
@Override
|
||||
public MxGatewayCli.MxGatewayCliSession session(String sessionId) {
|
||||
throw new UnsupportedOperationException();
|
||||
}
|
||||
|
||||
@Override
|
||||
public AcknowledgeAlarmReply acknowledgeAlarm(AcknowledgeAlarmRequest request) {
|
||||
throw new UnsupportedOperationException();
|
||||
}
|
||||
|
||||
@Override
|
||||
public MxGatewayAlarmFeedSubscription streamAlarms(
|
||||
StreamAlarmsRequest request, StreamObserver<AlarmFeedMessage> observer) {
|
||||
// Synchronously push 2000 messages to overflow the CLI's bounded
|
||||
// 1024-element queue. The CLI must surface the overflow rather
|
||||
// than silently dropping the trailing ~976 messages.
|
||||
for (int i = 0; i < 2000; i++) {
|
||||
observer.onNext(AlarmFeedMessage.newBuilder()
|
||||
.setActiveAlarm(ActiveAlarmSnapshot.newBuilder()
|
||||
.setAlarmFullReference("Flood." + i)
|
||||
.setCurrentState(AlarmConditionState.ALARM_CONDITION_STATE_ACTIVE)
|
||||
.setSeverity(700))
|
||||
.build());
|
||||
}
|
||||
observer.onCompleted();
|
||||
return new MxGatewayAlarmFeedSubscription();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void close() {
|
||||
}
|
||||
}
|
||||
|
||||
private static final class FakeClient implements MxGatewayCli.MxGatewayCliClient {
|
||||
private final PrintWriter out;
|
||||
private final FakeSession session = new FakeSession();
|
||||
|
||||
Reference in New Issue
Block a user