From c9f9b7b0afe7c4ba35c78fe3fd09d143c3dc08c4 Mon Sep 17 00:00:00 2001 From: Shinnosuke Suzuki Date: Tue, 21 Jul 2026 00:40:50 +0900 Subject: [PATCH 1/6] =?UTF-8?q?refactor(cli):=20content=20canonicalize=20?= =?UTF-8?q?=E3=81=AE=E5=AE=9F=E8=A1=8C=20mode=20=E3=82=92=20exclusive=20Ar?= =?UTF-8?q?gGroup=20=E3=81=A8=E5=9E=8B=E3=81=A7=E8=A1=A8=E7=8F=BE=E3=81=99?= =?UTF-8?q?=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --dry-run と --apply の静的排他を Picocli @ArgGroup に移し、call() 内の 手動 ParameterException / new CommandLine(this) を削除する。実行 mode は CanonicalizationExecutionMode で表し、未指定時は従来どおり dry-run を既定とする。 --- .../cli/ContentCanonicalizeCommand.java | 71 ++++++++++++++---- .../cli/ContentCanonicalizeCommandTest.java | 75 ++++++++++++++++++- 2 files changed, 131 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/net/sasasin/sreader/cli/ContentCanonicalizeCommand.java b/app/src/main/java/net/sasasin/sreader/cli/ContentCanonicalizeCommand.java index ba17347..1ea102d 100644 --- a/app/src/main/java/net/sasasin/sreader/cli/ContentCanonicalizeCommand.java +++ b/app/src/main/java/net/sasasin/sreader/cli/ContentCanonicalizeCommand.java @@ -3,6 +3,7 @@ import net.sasasin.sreader.domain.ContentCanonicalizationResult; import net.sasasin.sreader.service.canonicalization.ContentCanonicalizationMaintenanceService; import org.springframework.stereotype.Component; +import picocli.CommandLine.ArgGroup; import picocli.CommandLine.Command; import picocli.CommandLine.Option; @@ -16,13 +17,8 @@ public class ContentCanonicalizeCommand implements java.util.concurrent.Callable private final ContentCanonicalizationMaintenanceService service; - @Option( - names = "--dry-run", - description = "Report changes without modifying the database or files") - private boolean dryRun; - - @Option(names = "--apply", description = "Apply database changes and remove stale text files") - private boolean apply; + @ArgGroup(exclusive = true, multiplicity = "0..1", heading = "Execution mode:%n") + private ExecutionModeOptions executionMode; @Option( names = "--host", @@ -49,17 +45,19 @@ public ContentCanonicalizeCommand(ContentCanonicalizationMaintenanceService serv @Override public Integer call() { - if (dryRun && apply) { - throw new picocli.CommandLine.ParameterException( - new picocli.CommandLine(this), "--dry-run and --apply are mutually exclusive"); - } + CanonicalizationExecutionMode mode = executionMode(); ContentCanonicalizationResult result = service.canonicalize( - new ContentCanonicalizationMaintenanceService.Options(host, batchSize, limit, apply)); - print(result, apply ? "apply" : "dry-run"); + new ContentCanonicalizationMaintenanceService.Options( + host, batchSize, limit, mode.apply())); + print(result, mode.displayValue()); return result.hasFailures() ? 1 : 0; } + private CanonicalizationExecutionMode executionMode() { + return executionMode == null ? CanonicalizationExecutionMode.DRY_RUN : executionMode.mode(); + } + private void print(ContentCanonicalizationResult result, String mode) { var scan = result.scan(); var groups = result.groups(); @@ -80,4 +78,51 @@ private void print(ContentCanonicalizationResult result, String mode) { System.out.printf("failed_files=%d%n", files.failedFiles()); System.out.printf("failed_groups=%d%n", groups.failedGroups()); } + + /** CLI execution mode for content canonicalization (dry-run vs apply). */ + enum CanonicalizationExecutionMode { + DRY_RUN(false, "dry-run"), + APPLY(true, "apply"); + + private final boolean apply; + private final String displayValue; + + CanonicalizationExecutionMode(boolean apply, String displayValue) { + this.apply = apply; + this.displayValue = displayValue; + } + + boolean apply() { + return apply; + } + + String displayValue() { + return displayValue; + } + } + + /** + * Optional exclusive group for {@code --dry-run} / {@code --apply}. When the group is omitted, + * {@link CanonicalizationExecutionMode#DRY_RUN} is used. + */ + static final class ExecutionModeOptions { + + @Option( + names = "--dry-run", + description = "Report changes without modifying the database or files (default mode)") + boolean dryRun; + + @Option(names = "--apply", description = "Apply database changes and remove stale text files") + boolean apply; + + CanonicalizationExecutionMode mode() { + if (dryRun && apply) { + throw new IllegalStateException("Picocli exclusive group invariant violated"); + } + if (apply) { + return CanonicalizationExecutionMode.APPLY; + } + return CanonicalizationExecutionMode.DRY_RUN; + } + } } diff --git a/app/src/test/java/net/sasasin/sreader/cli/ContentCanonicalizeCommandTest.java b/app/src/test/java/net/sasasin/sreader/cli/ContentCanonicalizeCommandTest.java index 1e2ef1c..e35371b 100644 --- a/app/src/test/java/net/sasasin/sreader/cli/ContentCanonicalizeCommandTest.java +++ b/app/src/test/java/net/sasasin/sreader/cli/ContentCanonicalizeCommandTest.java @@ -4,10 +4,12 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import java.io.ByteArrayOutputStream; import java.io.PrintStream; +import java.io.PrintWriter; import java.nio.charset.StandardCharsets; import net.sasasin.sreader.domain.ContentCanonicalizationResult; import net.sasasin.sreader.domain.ContentCanonicalizationResult.DatabaseSummary; @@ -57,6 +59,27 @@ void defaultsToDryRunAndPrintsSummary() { "failed_groups=0"); } + @Test + void explicitDryRunUsesDryRunMode() { + ContentCanonicalizationMaintenanceService service = mock(); + when(service.canonicalize(any())).thenReturn(ContentCanonicalizationResult.empty()); + CommandLine cli = new CommandLine(new ContentCanonicalizeCommand(service)); + + ByteArrayOutputStream captured = new ByteArrayOutputStream(); + PrintStream originalOut = System.out; + System.setOut(new PrintStream(captured, true, StandardCharsets.UTF_8)); + try { + assertThat(cli.execute("--dry-run")).isZero(); + } finally { + System.setOut(originalOut); + } + + verify(service) + .canonicalize( + new ContentCanonicalizationMaintenanceService.Options(null, 100, null, false)); + assertThat(captured.toString(StandardCharsets.UTF_8)).contains("mode=dry-run"); + } + @Test void applyReturnsNonZeroForFailedFiles() { ContentCanonicalizationMaintenanceService service = mock(); @@ -108,15 +131,63 @@ void applyModeLabelIsPrinted() { } @Test - void rejectsConflictingModesAndInvalidNumbers() { + void rejectsConflictingModesWithoutCallingService() { + ContentCanonicalizationMaintenanceService service = mock(); + ByteArrayOutputStream err = new ByteArrayOutputStream(); + CommandLine applyThenDryRun = new CommandLine(new ContentCanonicalizeCommand(service)); + applyThenDryRun.setErr(new PrintWriter(err, true, StandardCharsets.UTF_8)); + assertThat(applyThenDryRun.execute("--apply", "--dry-run")).isEqualTo(2); + assertThat(err.toString(StandardCharsets.UTF_8)) + .containsIgnoringCase("mutually exclusive") + .contains("--apply") + .contains("--dry-run"); + verifyNoInteractions(service); + + ByteArrayOutputStream err2 = new ByteArrayOutputStream(); + CommandLine dryRunThenApply = new CommandLine(new ContentCanonicalizeCommand(service)); + dryRunThenApply.setErr(new PrintWriter(err2, true, StandardCharsets.UTF_8)); + assertThat(dryRunThenApply.execute("--dry-run", "--apply")).isEqualTo(2); + assertThat(err2.toString(StandardCharsets.UTF_8)) + .containsIgnoringCase("mutually exclusive") + .contains("--apply") + .contains("--dry-run"); + verifyNoInteractions(service); + } + + @Test + void invalidNumbersRemainExecutionErrors() { ContentCanonicalizationMaintenanceService service = mock(); CommandLine cli = new CommandLine(new ContentCanonicalizeCommand(service)); - assertThat(cli.execute("--apply", "--dry-run")).isEqualTo(2); assertThat(cli.execute("--batch-size", "0")).isEqualTo(1); assertThat(cli.execute("--limit", "0")).isEqualTo(1); } + @Test + void executionModeOptionsDefensiveInvariantRejectsBothFlags() { + ContentCanonicalizeCommand.ExecutionModeOptions options = + new ContentCanonicalizeCommand.ExecutionModeOptions(); + options.dryRun = true; + options.apply = true; + org.junit.jupiter.api.Assertions.assertThrows(IllegalStateException.class, options::mode); + } + + @Test + void helpShowsExecutionModeGroup() { + ContentCanonicalizationMaintenanceService service = mock(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + CommandLine cli = new CommandLine(new ContentCanonicalizeCommand(service)); + cli.setOut(new PrintWriter(out, true, StandardCharsets.UTF_8)); + + assertThat(cli.execute("--help")).isZero(); + String help = out.toString(StandardCharsets.UTF_8); + assertThat(help).contains("Execution mode:"); + assertThat(help).contains("--dry-run"); + assertThat(help).contains("--apply"); + // Flat Options section must not also list the group members a second time as root options. + assertThat(help).doesNotContain("Options:\n --dry-run"); + } + private ContentCanonicalizationResult result() { return new ContentCanonicalizationResult( new ScanSummary(2, 1), From f6a08096aa1a7cb8f4cbc8b89d18ae9d16898047 Mon Sep 17 00:00:00 2001 From: Shinnosuke Suzuki Date: Tue, 21 Jul 2026 00:41:00 +0900 Subject: [PATCH 2/6] =?UTF-8?q?feat(cli):=20feed=20entry=20selector=20?= =?UTF-8?q?=E7=94=A8=E3=81=AE=20Picocli=20converter=20=E3=82=92=E8=BF=BD?= =?UTF-8?q?=E5=8A=A0=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --entry / --entry-index / --entry-title-regex / --entry-url-regex の値を parse 段階で FeedEntrySelection へ変換する。blank・不正値は TypeConversionException として usage error にし、command 内の手動 parse へ持ち込まない。 --- .../cli/EntryIndexSelectionConverter.java | 28 ++++++ .../cli/EntryPositionSelectionConverter.java | 22 +++++ .../EntryTitleRegexSelectionConverter.java | 23 +++++ .../cli/EntryUrlRegexSelectionConverter.java | 23 +++++ .../cli/EntrySelectionConverterTest.java | 89 +++++++++++++++++++ 5 files changed, 185 insertions(+) create mode 100644 app/src/main/java/net/sasasin/sreader/cli/EntryIndexSelectionConverter.java create mode 100644 app/src/main/java/net/sasasin/sreader/cli/EntryPositionSelectionConverter.java create mode 100644 app/src/main/java/net/sasasin/sreader/cli/EntryTitleRegexSelectionConverter.java create mode 100644 app/src/main/java/net/sasasin/sreader/cli/EntryUrlRegexSelectionConverter.java create mode 100644 app/src/test/java/net/sasasin/sreader/cli/EntrySelectionConverterTest.java diff --git a/app/src/main/java/net/sasasin/sreader/cli/EntryIndexSelectionConverter.java b/app/src/main/java/net/sasasin/sreader/cli/EntryIndexSelectionConverter.java new file mode 100644 index 0000000..02197f3 --- /dev/null +++ b/app/src/main/java/net/sasasin/sreader/cli/EntryIndexSelectionConverter.java @@ -0,0 +1,28 @@ +package net.sasasin.sreader.cli; + +import net.sasasin.sreader.domain.FeedEntrySelection; +import picocli.CommandLine.ITypeConverter; +import picocli.CommandLine.TypeConversionException; + +/** Converts {@code --entry-index } to a {@link FeedEntrySelection}. */ +final class EntryIndexSelectionConverter implements ITypeConverter { + + @Override + public FeedEntrySelection convert(String value) { + if (value == null || value.isBlank()) { + throw new TypeConversionException("--entry-index must be a non-negative integer: " + value); + } + String trimmed = value.trim(); + final int index; + try { + index = Integer.parseInt(trimmed); + } catch (NumberFormatException e) { + throw new TypeConversionException("--entry-index must be a non-negative integer: " + value); + } + try { + return FeedEntrySelection.index(index); + } catch (IllegalArgumentException e) { + throw new TypeConversionException("--entry-index must be a non-negative integer: " + value); + } + } +} diff --git a/app/src/main/java/net/sasasin/sreader/cli/EntryPositionSelectionConverter.java b/app/src/main/java/net/sasasin/sreader/cli/EntryPositionSelectionConverter.java new file mode 100644 index 0000000..34e4306 --- /dev/null +++ b/app/src/main/java/net/sasasin/sreader/cli/EntryPositionSelectionConverter.java @@ -0,0 +1,22 @@ +package net.sasasin.sreader.cli; + +import java.util.Locale; +import net.sasasin.sreader.domain.FeedEntrySelection; +import picocli.CommandLine.ITypeConverter; +import picocli.CommandLine.TypeConversionException; + +/** Converts {@code --entry first|latest} to a {@link FeedEntrySelection}. */ +final class EntryPositionSelectionConverter implements ITypeConverter { + + @Override + public FeedEntrySelection convert(String value) { + if (value == null || value.isBlank()) { + throw new TypeConversionException("--entry must be first or latest"); + } + return switch (value.trim().toLowerCase(Locale.ROOT)) { + case "first" -> FeedEntrySelection.first(); + case "latest" -> FeedEntrySelection.latest(); + default -> throw new TypeConversionException("--entry must be first or latest: " + value); + }; + } +} diff --git a/app/src/main/java/net/sasasin/sreader/cli/EntryTitleRegexSelectionConverter.java b/app/src/main/java/net/sasasin/sreader/cli/EntryTitleRegexSelectionConverter.java new file mode 100644 index 0000000..69c614b --- /dev/null +++ b/app/src/main/java/net/sasasin/sreader/cli/EntryTitleRegexSelectionConverter.java @@ -0,0 +1,23 @@ +package net.sasasin.sreader.cli; + +import net.sasasin.sreader.domain.FeedEntrySelection; +import picocli.CommandLine.ITypeConverter; +import picocli.CommandLine.TypeConversionException; + +/** Converts {@code --entry-title-regex } to a {@link FeedEntrySelection}. */ +final class EntryTitleRegexSelectionConverter implements ITypeConverter { + + @Override + public FeedEntrySelection convert(String value) { + if (value == null || value.isBlank()) { + throw new TypeConversionException("--entry-title-regex must not be blank"); + } + try { + // Keep original value (no trim of pattern); domain validates syntax. + return FeedEntrySelection.titleRegex(value); + } catch (IllegalArgumentException e) { + throw new TypeConversionException( + "--entry-title-regex is not a valid regular expression: " + value); + } + } +} diff --git a/app/src/main/java/net/sasasin/sreader/cli/EntryUrlRegexSelectionConverter.java b/app/src/main/java/net/sasasin/sreader/cli/EntryUrlRegexSelectionConverter.java new file mode 100644 index 0000000..89f1393 --- /dev/null +++ b/app/src/main/java/net/sasasin/sreader/cli/EntryUrlRegexSelectionConverter.java @@ -0,0 +1,23 @@ +package net.sasasin.sreader.cli; + +import net.sasasin.sreader.domain.FeedEntrySelection; +import picocli.CommandLine.ITypeConverter; +import picocli.CommandLine.TypeConversionException; + +/** Converts {@code --entry-url-regex } to a {@link FeedEntrySelection}. */ +final class EntryUrlRegexSelectionConverter implements ITypeConverter { + + @Override + public FeedEntrySelection convert(String value) { + if (value == null || value.isBlank()) { + throw new TypeConversionException("--entry-url-regex must not be blank"); + } + try { + // Keep original value (no trim of pattern); domain validates syntax. + return FeedEntrySelection.urlRegex(value); + } catch (IllegalArgumentException e) { + throw new TypeConversionException( + "--entry-url-regex is not a valid regular expression: " + value); + } + } +} diff --git a/app/src/test/java/net/sasasin/sreader/cli/EntrySelectionConverterTest.java b/app/src/test/java/net/sasasin/sreader/cli/EntrySelectionConverterTest.java new file mode 100644 index 0000000..d3bc520 --- /dev/null +++ b/app/src/test/java/net/sasasin/sreader/cli/EntrySelectionConverterTest.java @@ -0,0 +1,89 @@ +package net.sasasin.sreader.cli; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import net.sasasin.sreader.domain.FeedEntrySelection; +import org.junit.jupiter.api.Test; +import picocli.CommandLine.TypeConversionException; + +class EntrySelectionConverterTest { + + private final EntryPositionSelectionConverter position = new EntryPositionSelectionConverter(); + private final EntryIndexSelectionConverter index = new EntryIndexSelectionConverter(); + private final EntryTitleRegexSelectionConverter title = new EntryTitleRegexSelectionConverter(); + private final EntryUrlRegexSelectionConverter url = new EntryUrlRegexSelectionConverter(); + + @Test + void positionConvertsFirstAndLatestCaseInsensitivelyWithTrim() { + assertThat(position.convert(" FiRsT ")).isEqualTo(FeedEntrySelection.first()); + assertThat(position.convert("LATEST")).isEqualTo(FeedEntrySelection.latest()); + } + + @Test + void positionRejectsBlankAndUnknown() { + assertThatThrownBy(() -> position.convert(null)) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry"); + assertThatThrownBy(() -> position.convert("")) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry"); + assertThatThrownBy(() -> position.convert("newest")) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry"); + } + + @Test + void indexConvertsNonNegativeIntegers() { + assertThat(index.convert("0")).isEqualTo(FeedEntrySelection.index(0)); + assertThat(index.convert(" 12 ")).isEqualTo(FeedEntrySelection.index(12)); + } + + @Test + void indexRejectsBlankNegativeNonIntegerAndOverflow() { + assertThatThrownBy(() -> index.convert(null)) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry-index"); + assertThatThrownBy(() -> index.convert("")) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry-index"); + assertThatThrownBy(() -> index.convert("-1")) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry-index"); + assertThatThrownBy(() -> index.convert("1.5")) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry-index"); + assertThatThrownBy(() -> index.convert("99999999999999999999")) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry-index"); + } + + @Test + void titleRegexKeepsOriginalPatternAndRejectsBlankOrInvalid() { + assertThat(title.convert("Release .*")).isEqualTo(FeedEntrySelection.titleRegex("Release .*")); + assertThatThrownBy(() -> title.convert(null)) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry-title-regex"); + assertThatThrownBy(() -> title.convert(" ")) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry-title-regex"); + assertThatThrownBy(() -> title.convert("[")) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry-title-regex"); + } + + @Test + void urlRegexKeepsOriginalPatternAndRejectsBlankOrInvalid() { + assertThat(url.convert("/posts/[0-9]+")) + .isEqualTo(FeedEntrySelection.urlRegex("/posts/[0-9]+")); + assertThatThrownBy(() -> url.convert(null)) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry-url-regex"); + assertThatThrownBy(() -> url.convert("")) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry-url-regex"); + assertThatThrownBy(() -> url.convert("*")) + .isInstanceOf(TypeConversionException.class) + .hasMessageContaining("--entry-url-regex"); + } +} From 550ca4ac245e17f436564706bb9f9f88d1254c49 Mon Sep 17 00:00:00 2001 From: Shinnosuke Suzuki Date: Tue, 21 Jul 2026 00:41:07 +0900 Subject: [PATCH 3/6] =?UTF-8?q?refactor(cli):=20feed=20probe=20=E3=81=AE?= =?UTF-8?q?=20entry=20selector=20=E3=82=92=20exclusive=20ArgGroup=20?= =?UTF-8?q?=E3=81=B8=E7=A7=BB=E8=A1=8C=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 4 つの selector を optional exclusive group にまとめ、複数指定を優先順位ではなく usage error にする。resolveSelection の priority chain と blank fall-through を廃止し、 未指定時のみ first を default とする。help からも selection priority 文言を削除する。 --- .../sasasin/sreader/cli/ProbeFeedCommand.java | 152 ++++++++--------- .../sreader/cli/ProbeFeedCommandTest.java | 156 ++++++++++++++---- 2 files changed, 199 insertions(+), 109 deletions(-) diff --git a/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCommand.java b/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCommand.java index c49491b..9508d14 100644 --- a/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCommand.java +++ b/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCommand.java @@ -1,25 +1,25 @@ package net.sasasin.sreader.cli; -import java.net.URI; -import java.util.Optional; +import java.util.List; +import java.util.Objects; import java.util.concurrent.Callable; +import java.util.stream.Stream; import net.sasasin.sreader.domain.FeedEntrySelection; import net.sasasin.sreader.domain.FullTextMethod; import net.sasasin.sreader.service.probe.FullTextProbeService; import net.sasasin.sreader.service.probe.ProbeOutcome; import org.springframework.stereotype.Component; +import picocli.CommandLine.ArgGroup; import picocli.CommandLine.Command; import picocli.CommandLine.Model.CommandSpec; import picocli.CommandLine.Option; -import picocli.CommandLine.ParameterException; import picocli.CommandLine.Spec; @Command( name = "feed", description = { "Fetch a feed, select one entry, extract full text with chosen method, print text.", - "Entry selection priority: --entry-index > --entry-title-regex > --entry-url-regex > --entry" - + " latest > --entry first (default first).", + "Choose at most one entry selector; default is first entry.", "For --method feed: uses feed entry content/desc (no article fetch). --xpath invalid with" + " feed method.", "STDOUT: body text. --verbose: diagnostics to STDERR." @@ -57,29 +57,11 @@ public class ProbeFeedCommand implements Callable { description = "XPath override (not for feed method)") private String xpath; - @Option( - names = "--entry", - paramLabel = "", - description = "Select first or latest entry (by date)") - private String entry; - - @Option( - names = "--entry-index", - paramLabel = "", - description = "0-based index of entry in feed") - private Integer entryIndex; - - @Option( - names = "--entry-title-regex", - paramLabel = "", - description = "Select first entry whose title matches regex") - private String entryTitleRegex; - - @Option( - names = "--entry-url-regex", - paramLabel = "", - description = "Select first entry whose link matches regex") - private String entryUrlRegex; + @ArgGroup( + exclusive = true, + multiplicity = "0..1", + heading = "Entry selection (choose at most one; default: first):%n") + private EntrySelectionOptions entrySelection; @Option(names = "--verbose", description = "Print diagnostics to STDERR") private boolean verbose; @@ -100,21 +82,15 @@ public ProbeFeedCommand(FullTextProbeService fullTextProbeService) { @Override public Integer call() { try { - URI uri = UrlValidator.validateHttpUrl(feedUrl, "--feed-url", spec); - - if (xpath != null && !xpath.isBlank() && !method.supportsXpathOverride()) { - throw new ParameterException( - spec.commandLine(), "--xpath cannot be used with --method feed"); - } - - FeedEntrySelection selection = resolveSelection(); - - Optional xp = - (xpath != null && !xpath.isBlank()) ? Optional.of(xpath) : Optional.empty(); - - ProbeOutcome outcome = fullTextProbeService.probeFeed(uri, method, selection, xp); - return mapOutcome(outcome); - } catch (ParameterException pe) { + ProbeFeedCliRequest request = + ProbeFeedCliRequest.create( + spec, feedUrl, method, selection(), xpath, verbose, output, maxChars); + + ProbeOutcome outcome = + fullTextProbeService.probeFeed( + request.feedUrl(), request.method(), request.selection(), request.xpath()); + return mapOutcome(outcome, request); + } catch (picocli.CommandLine.ParameterException pe) { throw pe; } catch (RuntimeException e) { spec.commandLine().getErr().println("Error: " + e.getMessage()); @@ -122,13 +98,22 @@ public Integer call() { } } - private int mapOutcome(ProbeOutcome outcome) { + private FeedEntrySelection selection() { + return entrySelection == null ? FeedEntrySelection.first() : entrySelection.selection(); + } + + private int mapOutcome(ProbeOutcome outcome, ProbeFeedCliRequest request) { ProbeOutputWriter writer = new ProbeOutputWriter(spec); return switch (outcome) { case ProbeOutcome.Succeeded succeeded -> - writer.writeSucceeded(succeeded.document(), succeeded.text(), verbose, output, maxChars); + writer.writeSucceeded( + succeeded.document(), + succeeded.text(), + request.verbose(), + request.output(), + request.maxChars()); case ProbeOutcome.NoContent noContent -> { - if (verbose) { + if (request.verbose()) { writer.writeNoContentDiagnostics(noContent.document()); } yield CliExitCodes.EMPTY_RESULT; @@ -155,42 +140,47 @@ private int mapOutcome(ProbeOutcome outcome) { }; } - private FeedEntrySelection resolveSelection() { - if (entryIndex != null) { - try { - return FeedEntrySelection.index(entryIndex); - } catch (IllegalArgumentException e) { - throw new ParameterException( - spec.commandLine(), "Invalid --entry-index value: " + e.getMessage()); - } - } - if (entryTitleRegex != null && !entryTitleRegex.isBlank()) { - try { - return FeedEntrySelection.titleRegex(entryTitleRegex); - } catch (IllegalArgumentException e) { - throw new ParameterException( - spec.commandLine(), "Invalid --entry-title-regex value: " + e.getMessage()); - } - } - if (entryUrlRegex != null && !entryUrlRegex.isBlank()) { - try { - return FeedEntrySelection.urlRegex(entryUrlRegex); - } catch (IllegalArgumentException e) { - throw new ParameterException( - spec.commandLine(), "Invalid --entry-url-regex value: " + e.getMessage()); - } - } - if (entry != null) { - String e = entry.trim().toLowerCase(); - if ("latest".equals(e)) { - return FeedEntrySelection.latest(); - } - if ("first".equals(e)) { - return FeedEntrySelection.first(); + /** + * Optional exclusive group of entry selectors. When the group is omitted, {@link + * FeedEntrySelection#first()} is used by the command. + */ + static final class EntrySelectionOptions { + + @Option( + names = "--entry", + converter = EntryPositionSelectionConverter.class, + paramLabel = "", + description = "Select first or latest entry (by date)") + FeedEntrySelection position; + + @Option( + names = "--entry-index", + converter = EntryIndexSelectionConverter.class, + paramLabel = "", + description = "0-based index of entry in feed") + FeedEntrySelection index; + + @Option( + names = "--entry-title-regex", + converter = EntryTitleRegexSelectionConverter.class, + paramLabel = "", + description = "Select first entry whose title matches regex") + FeedEntrySelection titleRegex; + + @Option( + names = "--entry-url-regex", + converter = EntryUrlRegexSelectionConverter.class, + paramLabel = "", + description = "Select first entry whose link matches regex") + FeedEntrySelection urlRegex; + + FeedEntrySelection selection() { + List specified = + Stream.of(position, index, titleRegex, urlRegex).filter(Objects::nonNull).toList(); + if (specified.size() != 1) { + throw new IllegalStateException("Picocli entry selector group invariant violated"); } - throw new ParameterException( - spec.commandLine(), "Invalid --entry value, must be first or latest: " + entry); + return specified.getFirst(); } - return FeedEntrySelection.first(); } } diff --git a/app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCommandTest.java b/app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCommandTest.java index 52a873b..50a7978 100644 --- a/app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCommandTest.java +++ b/app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCommandTest.java @@ -15,6 +15,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Optional; +import java.util.stream.Stream; import net.sasasin.sreader.domain.FeedEntrySelection; import net.sasasin.sreader.domain.FullTextMethod; import net.sasasin.sreader.service.extraction.NoContentReason; @@ -27,6 +28,9 @@ import net.sasasin.sreader.service.probe.ProbeSkipReason; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.mockito.ArgumentCaptor; import picocli.CommandLine; @@ -56,53 +60,108 @@ void passesDefaultFirstSelectionAndXpathToService() { } @Test - void resolvesEachSelectorAndItsPriority() { + void resolvesEachIndividualSelector() { assertSelection(FeedEntrySelection.first(), "--entry", " FiRsT "); assertSelection(FeedEntrySelection.latest(), "--entry", " LaTeSt "); + assertSelection(FeedEntrySelection.index(0), "--entry-index", "0"); assertSelection(FeedEntrySelection.index(2), "--entry-index", "2"); assertSelection( FeedEntrySelection.titleRegex("Release .*"), "--entry-title-regex", "Release .*"); assertSelection( FeedEntrySelection.urlRegex("/posts/[0-9]+"), "--entry-url-regex", "/posts/[0-9]+"); - assertSelection( - FeedEntrySelection.index(3), - "--entry-index", - "3", - "--entry-title-regex", - "title", - "--entry-url-regex", - "url", - "--entry", - "latest"); - assertSelection( - FeedEntrySelection.titleRegex("title"), - "--entry-title-regex", - "title", - "--entry-url-regex", - "url", - "--entry", - "latest"); - assertSelection( - FeedEntrySelection.urlRegex("url"), "--entry-url-regex", "url", "--entry", "latest"); + } + + @ParameterizedTest + @MethodSource("conflictingEntrySelectors") + void rejectsConflictingSelectorsWithoutCallingService( + String optionA, String valueA, String optionB, String valueB) { + FullTextProbeService service = mock(FullTextProbeService.class); + Harness harness = harness(service); + + assertThat(harness.execute(optionA, valueA, optionB, valueB)).isEqualTo(2); + assertThat(harness.stderr()) + .containsIgnoringCase("mutually exclusive") + .contains(optionA) + .contains(optionB); + verifyNoInteractions(service); + } + + static Stream conflictingEntrySelectors() { + return Stream.of( + Arguments.of("--entry", "first", "--entry-index", "0"), + Arguments.of("--entry", "latest", "--entry-title-regex", "title"), + Arguments.of("--entry", "first", "--entry-url-regex", "url"), + Arguments.of("--entry-index", "1", "--entry-title-regex", "title"), + Arguments.of("--entry-index", "1", "--entry-url-regex", "url"), + Arguments.of("--entry-title-regex", "title", "--entry-url-regex", "url")); + } + + @Test + void rejectsThreeAndFourSelectorsWithoutCallingService() { + FullTextProbeService service = mock(FullTextProbeService.class); + + Harness three = harness(service); + assertThat( + three.execute("--entry", "first", "--entry-index", "0", "--entry-title-regex", "title")) + .isEqualTo(2); + assertThat(three.stderr()).containsIgnoringCase("mutually exclusive"); + verifyNoInteractions(service); + + Harness four = harness(service); + assertThat( + four.execute( + "--entry", + "first", + "--entry-index", + "0", + "--entry-title-regex", + "title", + "--entry-url-regex", + "url")) + .isEqualTo(2); + assertThat(four.stderr()).containsIgnoringCase("mutually exclusive"); + verifyNoInteractions(service); } @Test - void blankRegexesFallThroughAndBlankXpathIsIgnored() { + void blankXpathIsIgnored() { FullTextProbeService service = serviceReturning(succeeded("body")); Harness harness = harness(service); - assertThat( - harness.execute( - "--entry-title-regex", " ", "--entry-url-regex", "url", "--xpath", " \t ")) - .isZero(); + assertThat(harness.execute("--xpath", " \t ")).isZero(); verify(service) .probeFeed( URI.create("https://example.com/feed.xml"), FullTextMethod.HTTP, - FeedEntrySelection.urlRegex("url"), + FeedEntrySelection.first(), Optional.empty()); } + @Test + void blankSelectorsAreUsageErrorsWithoutCallingService() { + FullTextProbeService service = mock(FullTextProbeService.class); + + Harness blankEntry = harness(service); + assertThat(blankEntry.execute("--entry", "")).isEqualTo(2); + assertThat(blankEntry.stderr()).contains("--entry"); + verifyNoInteractions(service); + + Harness blankIndex = harness(service); + assertThat(blankIndex.execute("--entry-index", "")).isEqualTo(2); + assertThat(blankIndex.stderr()).contains("--entry-index"); + verifyNoInteractions(service); + + Harness blankTitle = harness(service); + assertThat(blankTitle.execute("--entry-title-regex", " ")).isEqualTo(2); + assertThat(blankTitle.stderr()).contains("--entry-title-regex"); + verifyNoInteractions(service); + + Harness blankUrl = harness(service); + assertThat(blankUrl.execute("--entry-url-regex", " ")).isEqualTo(2); + assertThat(blankUrl.stderr()).contains("--entry-url-regex"); + verifyNoInteractions(service); + } + @Test void rejectsInvalidInputBeforeCallingService() { FullTextProbeService service = mock(FullTextProbeService.class); @@ -127,7 +186,7 @@ void rejectsInvalidInputBeforeCallingService() { } @Test - void rejectsNegativeIndexAndInvalidRegexAsUsageErrorsWithoutCallingService() { + void rejectsNegativeIndexOverflowAndInvalidRegexAsUsageErrorsWithoutCallingService() { FullTextProbeService service = mock(FullTextProbeService.class); Harness negativeIndex = harness(service); @@ -135,6 +194,16 @@ void rejectsNegativeIndexAndInvalidRegexAsUsageErrorsWithoutCallingService() { assertThat(negativeIndex.stderr()).contains("--entry-index"); verifyNoInteractions(service); + Harness nonInteger = harness(service); + assertThat(nonInteger.execute("--entry-index", "1.5")).isEqualTo(2); + assertThat(nonInteger.stderr()).contains("--entry-index"); + verifyNoInteractions(service); + + Harness overflow = harness(service); + assertThat(overflow.execute("--entry-index", "99999999999999999999")).isEqualTo(2); + assertThat(overflow.stderr()).contains("--entry-index"); + verifyNoInteractions(service); + Harness invalidTitle = harness(service); assertThat(invalidTitle.execute("--entry-title-regex", "[")).isEqualTo(2); assertThat(invalidTitle.stderr()).contains("--entry-title-regex"); @@ -146,6 +215,37 @@ void rejectsNegativeIndexAndInvalidRegexAsUsageErrorsWithoutCallingService() { verifyNoInteractions(service); } + @Test + void entrySelectionOptionsDefensiveInvariantRequiresExactlyOneMember() { + ProbeFeedCommand.EntrySelectionOptions empty = new ProbeFeedCommand.EntrySelectionOptions(); + org.junit.jupiter.api.Assertions.assertThrows(IllegalStateException.class, empty::selection); + + ProbeFeedCommand.EntrySelectionOptions multi = new ProbeFeedCommand.EntrySelectionOptions(); + multi.position = FeedEntrySelection.first(); + multi.index = FeedEntrySelection.index(0); + org.junit.jupiter.api.Assertions.assertThrows(IllegalStateException.class, multi::selection); + } + + @Test + void helpShowsEntrySelectionGroupWithoutPriorityWording() { + FullTextProbeService service = mock(FullTextProbeService.class); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + CommandLine cli = new CommandLine(new ProbeFeedCommand(service)); + cli.setOut(new PrintWriter(out, true, StandardCharsets.UTF_8)); + + assertThat(cli.execute("--help")).isZero(); + String help = out.toString(StandardCharsets.UTF_8); + assertThat(help).contains("Entry selection"); + assertThat(help).contains("choose at most one"); + assertThat(help).contains("default: first"); + assertThat(help).contains("--entry"); + assertThat(help).contains("--entry-index"); + assertThat(help).contains("--entry-title-regex"); + assertThat(help).contains("--entry-url-regex"); + assertThat(help.toLowerCase()).doesNotContain("selection priority"); + assertThat(help.toLowerCase()).doesNotContain("priority:"); + } + @Test void mapsServiceOutcomesToDocumentedExitCodes() { FullTextProbeService noMatch = mock(FullTextProbeService.class); From 83b67eea7b802cf370c750012fb11befd1cb5dcd Mon Sep 17 00:00:00 2001 From: Shinnosuke Suzuki Date: Tue, 21 Jul 2026 00:41:54 +0900 Subject: [PATCH 4/6] =?UTF-8?q?refactor(cli):=20probe=20=E3=81=AE=20method?= =?UTF-8?q?/xpath=20=E5=88=B6=E7=B4=84=E3=82=92=20typed=20CLI=20request=20?= =?UTF-8?q?=E3=81=AB=E9=9B=86=E7=B4=84=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit article/feed probe の URL 検証と FullTextMethod capability 判定を ProbeArticleCliRequest / ProbeFeedCliRequest の factory へ移し、command の call() を request 生成 → service 呼び出し → outcome mapping に限定する。 service 側の defensive validation は維持する。 --- .../sreader/cli/ProbeArticleCliRequest.java | 94 +++++++++++ .../sreader/cli/ProbeArticleCommand.java | 29 ++-- .../sreader/cli/ProbeFeedCliRequest.java | 101 ++++++++++++ .../cli/ProbeArticleCliRequestTest.java | 105 ++++++++++++ .../sreader/cli/ProbeArticleCommandTest.java | 33 ++++ .../sreader/cli/ProbeFeedCliRequestTest.java | 153 ++++++++++++++++++ 6 files changed, 499 insertions(+), 16 deletions(-) create mode 100644 app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCliRequest.java create mode 100644 app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCliRequest.java create mode 100644 app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCliRequestTest.java create mode 100644 app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCliRequestTest.java diff --git a/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCliRequest.java b/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCliRequest.java new file mode 100644 index 0000000..47cca91 --- /dev/null +++ b/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCliRequest.java @@ -0,0 +1,94 @@ +package net.sasasin.sreader.cli; + +import java.net.URI; +import java.util.Objects; +import java.util.Optional; +import net.sasasin.sreader.domain.FullTextMethod; +import picocli.CommandLine.Model.CommandSpec; +import picocli.CommandLine.ParameterException; + +/** + * Validated immutable CLI boundary for {@code probe article}. Invalid method/xpath combinations + * cannot be constructed. + */ +final class ProbeArticleCliRequest { + + private final URI url; + private final FullTextMethod method; + private final Optional xpath; + private final boolean verbose; + private final String output; + private final Integer maxChars; + + private ProbeArticleCliRequest( + URI url, + FullTextMethod method, + Optional xpath, + boolean verbose, + String output, + Integer maxChars) { + this.url = Objects.requireNonNull(url, "url"); + this.method = Objects.requireNonNull(method, "method"); + this.xpath = Objects.requireNonNull(xpath, "xpath"); + this.verbose = verbose; + this.output = output; + this.maxChars = maxChars; + } + + static ProbeArticleCliRequest create( + CommandSpec spec, + String url, + FullTextMethod method, + String xpath, + boolean verbose, + String output, + Integer maxChars) { + Objects.requireNonNull(spec, "spec"); + if (method == null) { + throw new ParameterException(spec.commandLine(), "--method is required"); + } + if (!method.supportsArticleProbe()) { + throw new ParameterException( + spec.commandLine(), "--method feed is not valid for 'probe article'"); + } + URI validatedUrl = UrlValidator.validateHttpUrl(url, "--url", spec); + Optional normalizedXpath = normalizeXpath(xpath); + if (normalizedXpath.isPresent() && !method.supportsXpathOverride()) { + throw new ParameterException( + spec.commandLine(), "--xpath cannot be used with --method " + method.value()); + } + return new ProbeArticleCliRequest( + validatedUrl, method, normalizedXpath, verbose, output, maxChars); + } + + URI url() { + return url; + } + + FullTextMethod method() { + return method; + } + + Optional xpath() { + return xpath; + } + + boolean verbose() { + return verbose; + } + + String output() { + return output; + } + + Integer maxChars() { + return maxChars; + } + + private static Optional normalizeXpath(String xpath) { + if (xpath == null || xpath.isBlank()) { + return Optional.empty(); + } + return Optional.of(xpath); + } +} diff --git a/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCommand.java b/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCommand.java index f8a1c81..d0a9731 100644 --- a/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCommand.java +++ b/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCommand.java @@ -1,7 +1,5 @@ package net.sasasin.sreader.cli; -import java.net.URI; -import java.util.Optional; import java.util.concurrent.Callable; import net.sasasin.sreader.domain.FullTextMethod; import net.sasasin.sreader.service.probe.FullTextProbeService; @@ -76,18 +74,12 @@ public ProbeArticleCommand(FullTextProbeService fullTextProbeService) { @Override public Integer call() { try { - URI uri = UrlValidator.validateHttpUrl(url, "--url", spec); + ProbeArticleCliRequest request = + ProbeArticleCliRequest.create(spec, url, method, xpath, verbose, output, maxChars); - if (!method.supportsArticleProbe()) { - throw new picocli.CommandLine.ParameterException( - spec.commandLine(), "--method feed is not valid for 'probe article'"); - } - - Optional xp = - (xpath != null && !xpath.isBlank()) ? Optional.of(xpath) : Optional.empty(); - - ProbeOutcome outcome = fullTextProbeService.probeArticle(uri, method, xp); - return mapOutcome(outcome); + ProbeOutcome outcome = + fullTextProbeService.probeArticle(request.url(), request.method(), request.xpath()); + return mapOutcome(outcome, request); } catch (picocli.CommandLine.ParameterException pe) { throw pe; } catch (RuntimeException e) { @@ -96,13 +88,18 @@ public Integer call() { } } - private int mapOutcome(ProbeOutcome outcome) { + private int mapOutcome(ProbeOutcome outcome, ProbeArticleCliRequest request) { ProbeOutputWriter writer = new ProbeOutputWriter(spec); return switch (outcome) { case ProbeOutcome.Succeeded succeeded -> - writer.writeSucceeded(succeeded.document(), succeeded.text(), verbose, output, maxChars); + writer.writeSucceeded( + succeeded.document(), + succeeded.text(), + request.verbose(), + request.output(), + request.maxChars()); case ProbeOutcome.NoContent noContent -> { - if (verbose) { + if (request.verbose()) { writer.writeNoContentDiagnostics(noContent.document()); } yield CliExitCodes.EMPTY_RESULT; diff --git a/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCliRequest.java b/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCliRequest.java new file mode 100644 index 0000000..fa9ad62 --- /dev/null +++ b/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCliRequest.java @@ -0,0 +1,101 @@ +package net.sasasin.sreader.cli; + +import java.net.URI; +import java.util.Objects; +import java.util.Optional; +import net.sasasin.sreader.domain.FeedEntrySelection; +import net.sasasin.sreader.domain.FullTextMethod; +import picocli.CommandLine.Model.CommandSpec; +import picocli.CommandLine.ParameterException; + +/** + * Validated immutable CLI boundary for {@code probe feed}. Invalid method/xpath combinations and + * missing selection cannot be constructed. + */ +final class ProbeFeedCliRequest { + + private final URI feedUrl; + private final FullTextMethod method; + private final FeedEntrySelection selection; + private final Optional xpath; + private final boolean verbose; + private final String output; + private final Integer maxChars; + + private ProbeFeedCliRequest( + URI feedUrl, + FullTextMethod method, + FeedEntrySelection selection, + Optional xpath, + boolean verbose, + String output, + Integer maxChars) { + this.feedUrl = Objects.requireNonNull(feedUrl, "feedUrl"); + this.method = Objects.requireNonNull(method, "method"); + this.selection = Objects.requireNonNull(selection, "selection"); + this.xpath = Objects.requireNonNull(xpath, "xpath"); + this.verbose = verbose; + this.output = output; + this.maxChars = maxChars; + } + + static ProbeFeedCliRequest create( + CommandSpec spec, + String feedUrl, + FullTextMethod method, + FeedEntrySelection selection, + String xpath, + boolean verbose, + String output, + Integer maxChars) { + Objects.requireNonNull(spec, "spec"); + if (method == null) { + throw new ParameterException(spec.commandLine(), "--method is required"); + } + if (selection == null) { + throw new ParameterException(spec.commandLine(), "entry selection is required"); + } + URI validatedUrl = UrlValidator.validateHttpUrl(feedUrl, "--feed-url", spec); + Optional normalizedXpath = normalizeXpath(xpath); + if (normalizedXpath.isPresent() && !method.supportsXpathOverride()) { + throw new ParameterException(spec.commandLine(), "--xpath cannot be used with --method feed"); + } + return new ProbeFeedCliRequest( + validatedUrl, method, selection, normalizedXpath, verbose, output, maxChars); + } + + URI feedUrl() { + return feedUrl; + } + + FullTextMethod method() { + return method; + } + + FeedEntrySelection selection() { + return selection; + } + + Optional xpath() { + return xpath; + } + + boolean verbose() { + return verbose; + } + + String output() { + return output; + } + + Integer maxChars() { + return maxChars; + } + + private static Optional normalizeXpath(String xpath) { + if (xpath == null || xpath.isBlank()) { + return Optional.empty(); + } + return Optional.of(xpath); + } +} diff --git a/app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCliRequestTest.java b/app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCliRequestTest.java new file mode 100644 index 0000000..c54cc24 --- /dev/null +++ b/app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCliRequestTest.java @@ -0,0 +1,105 @@ +package net.sasasin.sreader.cli; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.net.URI; +import net.sasasin.sreader.domain.FullTextMethod; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +import picocli.CommandLine; +import picocli.CommandLine.Command; +import picocli.CommandLine.Model.CommandSpec; +import picocli.CommandLine.ParameterException; + +class ProbeArticleCliRequestTest { + + @Command(name = "probe-article-request-test") + private static final class DummyCommand {} + + private final CommandSpec spec = new CommandLine(new DummyCommand()).getCommandSpec(); + + @ParameterizedTest + @EnumSource( + value = FullTextMethod.class, + names = {"FEED"}, + mode = EnumSource.Mode.EXCLUDE) + void acceptsArticleCapableMethods(FullTextMethod method) { + ProbeArticleCliRequest request = + ProbeArticleCliRequest.create( + spec, "https://example.com/a", method, null, false, null, null); + + assertThat(request.url()).isEqualTo(URI.create("https://example.com/a")); + assertThat(request.method()).isEqualTo(method); + assertThat(request.xpath()).isEmpty(); + } + + @Test + void rejectsFeedMethod() { + assertThatThrownBy( + () -> + ProbeArticleCliRequest.create( + spec, "https://example.com/a", FullTextMethod.FEED, null, false, null, null)) + .isInstanceOf(ParameterException.class) + .hasMessageContaining("--method feed"); + } + + @Test + void rejectsNullMethod() { + assertThatThrownBy( + () -> + ProbeArticleCliRequest.create( + spec, "https://example.com/a", null, null, false, null, null)) + .isInstanceOf(ParameterException.class) + .hasMessageContaining("--method"); + } + + @Test + void blankXpathIsAbsence() { + ProbeArticleCliRequest request = + ProbeArticleCliRequest.create( + spec, "https://example.com/a", FullTextMethod.HTTP, " \t ", true, "out.txt", 10); + + assertThat(request.xpath()).isEmpty(); + assertThat(request.verbose()).isTrue(); + assertThat(request.output()).isEqualTo("out.txt"); + assertThat(request.maxChars()).isEqualTo(10); + } + + @Test + void nonblankXpathIsKeptForSupportingMethods() { + ProbeArticleCliRequest request = + ProbeArticleCliRequest.create( + spec, "https://example.com/a", FullTextMethod.HTTP, "//article", false, null, null); + + assertThat(request.xpath()).contains("//article"); + } + + @Test + void rejectsInvalidUrlUsingCommandSpec() { + assertThatThrownBy( + () -> + ProbeArticleCliRequest.create( + spec, "not-a-url", FullTextMethod.HTTP, null, false, null, null)) + .isInstanceOf(ParameterException.class) + .satisfies( + ex -> { + ParameterException pe = (ParameterException) ex; + assertThat(pe.getCommandLine()).isSameAs(spec.commandLine()); + assertThat(pe.getMessage()).containsIgnoringCase("http"); + }); + } + + @Test + void parameterExceptionUsesInjectedCommandLine() { + try { + ProbeArticleCliRequest.create( + spec, "https://example.com/a", FullTextMethod.FEED, null, false, null, null); + } catch (ParameterException pe) { + assertThat(pe.getCommandLine()).isSameAs(spec.commandLine()); + return; + } + throw new AssertionError("expected ParameterException"); + } +} diff --git a/app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCommandTest.java b/app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCommandTest.java index 44a81cd..5818f70 100644 --- a/app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCommandTest.java +++ b/app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCommandTest.java @@ -190,6 +190,39 @@ void failedMapsToExitOneWithErrorPrefix() { assertThat(harness.stderr()).contains("Error: boom"); } + @Test + void interruptedFailureRestoresInterruptFlagAndReturnsOne() { + FullTextProbeService service = mock(FullTextProbeService.class); + when(service.probeArticle(any(), any(), any())) + .thenReturn( + new ProbeOutcome.Failed( + OperationFailure.of( + FailureStage.FETCH_ARTICLE, + FailureKind.INTERRUPTED, + "https://x", + "interrupted"))); + Harness harness = harness(service); + Thread.interrupted(); // clear any previous flag + try { + assertThat(harness.execute()).isEqualTo(1); + assertThat(Thread.currentThread().isInterrupted()).isTrue(); + assertThat(harness.stderr()).contains("Error: interrupted"); + } finally { + Thread.interrupted(); // clear for other tests + } + } + + @Test + void noMatchingEntryMapsToExitThree() { + FullTextProbeService service = mock(FullTextProbeService.class); + when(service.probeArticle(any(), any(), any())) + .thenReturn(new ProbeOutcome.NoMatchingEntry("none")); + Harness harness = harness(service); + + assertThat(harness.execute()).isEqualTo(3); + assertThat(harness.stderr()).contains("No matching feed entry: none"); + } + @Test void genericRuntimeExceptionMapsToExitOneWithErrorPrefix() { FullTextProbeService service = mock(FullTextProbeService.class); diff --git a/app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCliRequestTest.java b/app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCliRequestTest.java new file mode 100644 index 0000000..6df2bea --- /dev/null +++ b/app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCliRequestTest.java @@ -0,0 +1,153 @@ +package net.sasasin.sreader.cli; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.net.URI; +import net.sasasin.sreader.domain.FeedEntrySelection; +import net.sasasin.sreader.domain.FullTextMethod; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +import picocli.CommandLine; +import picocli.CommandLine.Command; +import picocli.CommandLine.Model.CommandSpec; +import picocli.CommandLine.ParameterException; + +class ProbeFeedCliRequestTest { + + @Command(name = "probe-feed-request-test") + private static final class DummyCommand {} + + private final CommandSpec spec = new CommandLine(new DummyCommand()).getCommandSpec(); + + @ParameterizedTest + @EnumSource(FullTextMethod.class) + void acceptsAllMethodsWithoutXpath(FullTextMethod method) { + ProbeFeedCliRequest request = + ProbeFeedCliRequest.create( + spec, + "https://example.com/feed.xml", + method, + FeedEntrySelection.first(), + null, + false, + null, + null); + + assertThat(request.feedUrl()).isEqualTo(URI.create("https://example.com/feed.xml")); + assertThat(request.method()).isEqualTo(method); + assertThat(request.selection()).isEqualTo(FeedEntrySelection.first()); + assertThat(request.xpath()).isEmpty(); + } + + @Test + void rejectsFeedMethodWithXpath() { + assertThatThrownBy( + () -> + ProbeFeedCliRequest.create( + spec, + "https://example.com/feed.xml", + FullTextMethod.FEED, + FeedEntrySelection.latest(), + "//p", + false, + null, + null)) + .isInstanceOf(ParameterException.class) + .hasMessageContaining("--xpath") + .hasMessageContaining("feed"); + } + + @Test + void acceptsArticleMethodWithXpath() { + ProbeFeedCliRequest request = + ProbeFeedCliRequest.create( + spec, + "https://example.com/feed.xml", + FullTextMethod.HTTP, + FeedEntrySelection.index(1), + "//article", + true, + "out.txt", + 20); + + assertThat(request.xpath()).contains("//article"); + assertThat(request.selection()).isEqualTo(FeedEntrySelection.index(1)); + assertThat(request.verbose()).isTrue(); + assertThat(request.output()).isEqualTo("out.txt"); + assertThat(request.maxChars()).isEqualTo(20); + } + + @Test + void blankXpathIsAbsenceEvenWithFeedMethod() { + ProbeFeedCliRequest request = + ProbeFeedCliRequest.create( + spec, + "https://example.com/feed.xml", + FullTextMethod.FEED, + FeedEntrySelection.first(), + " ", + false, + null, + null); + + assertThat(request.xpath()).isEmpty(); + } + + @Test + void rejectsNullSelection() { + assertThatThrownBy( + () -> + ProbeFeedCliRequest.create( + spec, + "https://example.com/feed.xml", + FullTextMethod.HTTP, + null, + null, + false, + null, + null)) + .isInstanceOf(ParameterException.class) + .hasMessageContaining("selection"); + } + + @Test + void rejectsNullMethod() { + assertThatThrownBy( + () -> + ProbeFeedCliRequest.create( + spec, + "https://example.com/feed.xml", + null, + FeedEntrySelection.first(), + null, + false, + null, + null)) + .isInstanceOf(ParameterException.class) + .hasMessageContaining("--method"); + } + + @Test + void rejectsInvalidFeedUrlUsingCommandSpec() { + assertThatThrownBy( + () -> + ProbeFeedCliRequest.create( + spec, + "not-a-url", + FullTextMethod.HTTP, + FeedEntrySelection.first(), + null, + false, + null, + null)) + .isInstanceOf(ParameterException.class) + .satisfies( + ex -> { + ParameterException pe = (ParameterException) ex; + assertThat(pe.getCommandLine()).isSameAs(spec.commandLine()); + assertThat(pe.getMessage()).containsIgnoringCase("http"); + }); + } +} From 49a28609633253a405bcb6d6c5c646e96e39fb3e Mon Sep 17 00:00:00 2001 From: Shinnosuke Suzuki Date: Tue, 21 Jul 2026 00:41:54 +0900 Subject: [PATCH 5/6] =?UTF-8?q?test(cli):=20exclusive=20option=20=E3=81=A8?= =?UTF-8?q?=20typed=20request=20=E3=81=AE=20regression=20=E3=82=92?= =?UTF-8?q?=E5=9B=BA=E5=AE=9A=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @ArgGroup の exclusive/multiplicity を reflection で検証し、手動 exclusivity や resolveSelection / selection priority の再導入を source guard で防ぐ。 SreaderCliExecutor 経由でも mode/selector conflict と dynamic capability が exit code 2 かつ service 未呼び出しになることを確認する。 --- .../CliOptionConstraintArchitectureTest.java | 123 ++++++++++++++++++ .../sreader/cli/SreaderCliExecutorTest.java | 105 ++++++++++++++- 2 files changed, 225 insertions(+), 3 deletions(-) create mode 100644 app/src/test/java/net/sasasin/sreader/cli/CliOptionConstraintArchitectureTest.java diff --git a/app/src/test/java/net/sasasin/sreader/cli/CliOptionConstraintArchitectureTest.java b/app/src/test/java/net/sasasin/sreader/cli/CliOptionConstraintArchitectureTest.java new file mode 100644 index 0000000..6072a15 --- /dev/null +++ b/app/src/test/java/net/sasasin/sreader/cli/CliOptionConstraintArchitectureTest.java @@ -0,0 +1,123 @@ +package net.sasasin.sreader.cli; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.lang.reflect.Field; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.Set; +import java.util.stream.Collectors; +import org.junit.jupiter.api.Test; +import picocli.CommandLine.ArgGroup; +import picocli.CommandLine.Option; + +/** + * Guards that exclusive CLI option relations stay expressed via Picocli {@link ArgGroup} and typed + * request factories, not manual priority chains or {@code new CommandLine(this)}. + */ +class CliOptionConstraintArchitectureTest { + + @Test + void contentCanonicalizeUsesOptionalExclusiveExecutionModeGroup() { + Field groupField = findArgGroupField(ContentCanonicalizeCommand.class); + ArgGroup argGroup = groupField.getAnnotation(ArgGroup.class); + assertThat(argGroup.exclusive()).isTrue(); + assertThat(argGroup.multiplicity()).isEqualTo("0..1"); + assertThat(optionNames(groupField.getType())).containsExactlyInAnyOrder("--dry-run", "--apply"); + } + + @Test + void probeFeedUsesOptionalExclusiveEntrySelectionGroup() { + Field groupField = findArgGroupField(ProbeFeedCommand.class); + ArgGroup argGroup = groupField.getAnnotation(ArgGroup.class); + assertThat(argGroup.exclusive()).isTrue(); + assertThat(argGroup.multiplicity()).isEqualTo("0..1"); + assertThat(optionNames(groupField.getType())) + .containsExactlyInAnyOrder( + "--entry", "--entry-index", "--entry-title-regex", "--entry-url-regex"); + } + + @Test + void contentCanonicalizeSourceDoesNotReintroduceManualModeValidation() throws IOException { + String source = readMain("cli/ContentCanonicalizeCommand.java"); + assertThat(source).contains("@ArgGroup"); + assertThat(source).contains("exclusive = true"); + assertThat(source).contains("multiplicity = \"0..1\""); + // User-input mutual exclusivity must not be reimplemented as ParameterException. + assertThat(source).doesNotContain("--dry-run and --apply are mutually exclusive"); + assertThat(source).doesNotContain("new CommandLine(this)"); + assertThat(source).doesNotContain("new picocli.CommandLine(this)"); + assertThat(source).doesNotContain("private boolean dryRun"); + assertThat(source).doesNotContain("private boolean apply"); + } + + @Test + void probeFeedSourceDoesNotReintroducePrioritySelection() throws IOException { + String source = readMain("cli/ProbeFeedCommand.java"); + assertThat(source).contains("@ArgGroup"); + assertThat(source).contains("exclusive = true"); + assertThat(source).contains("multiplicity = \"0..1\""); + assertThat(source).doesNotContain("resolveSelection"); + assertThat(source.toLowerCase()).doesNotContain("selection priority"); + assertThat(source).doesNotContain("private String entry;"); + assertThat(source).doesNotContain("private Integer entryIndex;"); + assertThat(source).doesNotContain("private String entryTitleRegex;"); + assertThat(source).doesNotContain("private String entryUrlRegex;"); + assertThat(source).contains("ProbeFeedCliRequest"); + } + + @Test + void probeArticleUsesTypedRequestFactory() throws IOException { + String source = readMain("cli/ProbeArticleCommand.java"); + assertThat(source).contains("ProbeArticleCliRequest"); + assertThat(source).doesNotContain("supportsArticleProbe()"); + assertThat(mainJavaRoot().resolve("cli/ProbeArticleCliRequest.java")).isRegularFile(); + assertThat(mainJavaRoot().resolve("cli/ProbeFeedCliRequest.java")).isRegularFile(); + } + + @Test + void feedImportDoesNotExclusiveGroupDryRunAndResubscribe() throws IOException { + String source = readMain("cli/FeedImportCommand.java"); + assertThat(source).doesNotContain("@ArgGroup"); + assertThat(source).contains("--dry-run"); + assertThat(source).contains("--resubscribe"); + } + + private static Field findArgGroupField(Class commandClass) { + Field[] fields = + Arrays.stream(commandClass.getDeclaredFields()) + .filter(field -> field.getAnnotation(ArgGroup.class) != null) + .toArray(Field[]::new); + assertThat(fields).as("exactly one @ArgGroup on %s", commandClass.getSimpleName()).hasSize(1); + return fields[0]; + } + + private static Set optionNames(Class groupType) { + return Arrays.stream(groupType.getDeclaredFields()) + .map(field -> field.getAnnotation(Option.class)) + .filter(option -> option != null) + .flatMap(option -> Arrays.stream(option.names())) + .collect(Collectors.toCollection(LinkedHashSet::new)); + } + + private static String readMain(String relative) throws IOException { + return Files.readString(mainJavaRoot().resolve(relative), StandardCharsets.UTF_8); + } + + private static Path mainJavaRoot() { + Path cwd = Path.of("").toAbsolutePath().normalize(); + Path fromAppModule = cwd.resolve("src/main/java/net/sasasin/sreader"); + if (Files.isDirectory(fromAppModule)) { + return fromAppModule; + } + Path fromRepoRoot = cwd.resolve("app/src/main/java/net/sasasin/sreader"); + if (Files.isDirectory(fromRepoRoot)) { + return fromRepoRoot; + } + throw new AssertionError("main java root not found from working directory: " + cwd); + } +} diff --git a/app/src/test/java/net/sasasin/sreader/cli/SreaderCliExecutorTest.java b/app/src/test/java/net/sasasin/sreader/cli/SreaderCliExecutorTest.java index c5ee51d..431ec94 100644 --- a/app/src/test/java/net/sasasin/sreader/cli/SreaderCliExecutorTest.java +++ b/app/src/test/java/net/sasasin/sreader/cli/SreaderCliExecutorTest.java @@ -4,8 +4,10 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import net.sasasin.sreader.scheduler.FeedReaderScheduler; +import net.sasasin.sreader.service.canonicalization.ContentCanonicalizationMaintenanceService; import net.sasasin.sreader.service.feed.FeedDiscoveryService; import net.sasasin.sreader.service.feed.toml.FeedTomlService; import net.sasasin.sreader.service.probe.FullTextProbeService; @@ -40,19 +42,110 @@ void runOnceRuntimeExceptionMapsToOne() { assertEquals(1, executor(scheduler).execute("run-once")); } + @Test + void contentCanonicalizeModeConflictIsUsageErrorWithoutCallingService() { + ContentCanonicalizationMaintenanceService maintenance = mock(); + FullTextProbeService probe = mock(); + SreaderCliExecutor executor = executor(mock(FeedReaderScheduler.class), maintenance, probe); + + assertEquals(2, executor.execute("content", "canonicalize", "--dry-run", "--apply")); + verifyNoInteractions(maintenance); + } + + @Test + void probeFeedSelectorConflictIsUsageErrorWithoutCallingService() { + FullTextProbeService probe = mock(); + SreaderCliExecutor executor = + executor( + mock(FeedReaderScheduler.class), + mock(ContentCanonicalizationMaintenanceService.class), + probe); + + assertEquals( + 2, + executor.execute( + "probe", + "feed", + "--feed-url", + "https://example.com/feed.xml", + "--method", + "http", + "--entry", + "first", + "--entry-index", + "0")); + verifyNoInteractions(probe); + } + + @Test + void probeFeedInvalidSelectorConversionIsUsageErrorWithoutCallingService() { + FullTextProbeService probe = mock(); + SreaderCliExecutor executor = + executor( + mock(FeedReaderScheduler.class), + mock(ContentCanonicalizationMaintenanceService.class), + probe); + + assertEquals( + 2, + executor.execute( + "probe", + "feed", + "--feed-url", + "https://example.com/feed.xml", + "--method", + "http", + "--entry", + "newest")); + verifyNoInteractions(probe); + } + + @Test + void probeArticleFeedMethodIsUsageErrorWithoutCallingService() { + FullTextProbeService probe = mock(); + SreaderCliExecutor executor = + executor( + mock(FeedReaderScheduler.class), + mock(ContentCanonicalizationMaintenanceService.class), + probe); + + assertEquals( + 2, + executor.execute("probe", "article", "--url", "https://example.com/a", "--method", "feed")); + verifyNoInteractions(probe); + } + private SreaderCliExecutor executor(FeedReaderScheduler scheduler) { - return new SreaderCliExecutor(new SreaderCommand(), new TestPicocliFactory(scheduler)); + return executor( + scheduler, + mock(ContentCanonicalizationMaintenanceService.class), + mock(FullTextProbeService.class)); + } + + private SreaderCliExecutor executor( + FeedReaderScheduler scheduler, + ContentCanonicalizationMaintenanceService maintenanceService, + FullTextProbeService fullTextProbeService) { + return new SreaderCliExecutor( + new SreaderCommand(), + new TestPicocliFactory(scheduler, maintenanceService, fullTextProbeService)); } private static final class TestPicocliFactory extends PicocliFactory { private final FeedReaderScheduler scheduler; + private final ContentCanonicalizationMaintenanceService maintenanceService; + private final FullTextProbeService fullTextProbeService; private final FeedTomlService feedTomlService = mock(FeedTomlService.class); private final FeedDiscoveryService feedDiscoveryService = mock(FeedDiscoveryService.class); - private final FullTextProbeService fullTextProbeService = mock(FullTextProbeService.class); - TestPicocliFactory(FeedReaderScheduler scheduler) { + TestPicocliFactory( + FeedReaderScheduler scheduler, + ContentCanonicalizationMaintenanceService maintenanceService, + FullTextProbeService fullTextProbeService) { super(null); this.scheduler = scheduler; + this.maintenanceService = maintenanceService; + this.fullTextProbeService = fullTextProbeService; } @Override @@ -69,6 +162,12 @@ public K create(Class cls) throws Exception { if (cls == ProbeFeedCommand.class) { return cls.cast(new ProbeFeedCommand(fullTextProbeService)); } + if (cls == ContentCommand.class) { + return cls.cast(new ContentCommand()); + } + if (cls == ContentCanonicalizeCommand.class) { + return cls.cast(new ContentCanonicalizeCommand(maintenanceService)); + } if (cls == FeedsDiscoverCommand.class) { return cls.cast(new FeedsDiscoverCommand(feedDiscoveryService)); } From 46a6d3de23c20ce0602f7e14162972b081dc5840 Mon Sep 17 00:00:00 2001 From: Shinnosuke Suzuki Date: Tue, 21 Jul 2026 00:58:14 +0900 Subject: [PATCH 6/6] Represent optional probe CLI outputs with non-null Optionals --- .../sreader/cli/ProbeArticleCliRequest.java | 23 +++++++++++------- .../sreader/cli/ProbeArticleCommand.java | 4 ++-- .../sreader/cli/ProbeFeedCliRequest.java | 24 ++++++++++++------- .../sasasin/sreader/cli/ProbeFeedCommand.java | 4 ++-- .../CliOptionConstraintArchitectureTest.java | 15 ++++++++++++ .../cli/ProbeArticleCliRequestTest.java | 6 +++-- .../sreader/cli/ProbeFeedCliRequestTest.java | 6 +++-- 7 files changed, 56 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCliRequest.java b/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCliRequest.java index 47cca91..43ebb97 100644 --- a/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCliRequest.java +++ b/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCliRequest.java @@ -17,22 +17,22 @@ final class ProbeArticleCliRequest { private final FullTextMethod method; private final Optional xpath; private final boolean verbose; - private final String output; - private final Integer maxChars; + private final Optional output; + private final Optional maxChars; private ProbeArticleCliRequest( URI url, FullTextMethod method, Optional xpath, boolean verbose, - String output, - Integer maxChars) { + Optional output, + Optional maxChars) { this.url = Objects.requireNonNull(url, "url"); this.method = Objects.requireNonNull(method, "method"); this.xpath = Objects.requireNonNull(xpath, "xpath"); this.verbose = verbose; - this.output = output; - this.maxChars = maxChars; + this.output = Objects.requireNonNull(output, "output"); + this.maxChars = Objects.requireNonNull(maxChars, "maxChars"); } static ProbeArticleCliRequest create( @@ -58,7 +58,12 @@ static ProbeArticleCliRequest create( spec.commandLine(), "--xpath cannot be used with --method " + method.value()); } return new ProbeArticleCliRequest( - validatedUrl, method, normalizedXpath, verbose, output, maxChars); + validatedUrl, + method, + normalizedXpath, + verbose, + Optional.ofNullable(output), + Optional.ofNullable(maxChars)); } URI url() { @@ -77,11 +82,11 @@ boolean verbose() { return verbose; } - String output() { + Optional output() { return output; } - Integer maxChars() { + Optional maxChars() { return maxChars; } diff --git a/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCommand.java b/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCommand.java index d0a9731..ef4f07a 100644 --- a/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCommand.java +++ b/app/src/main/java/net/sasasin/sreader/cli/ProbeArticleCommand.java @@ -96,8 +96,8 @@ private int mapOutcome(ProbeOutcome outcome, ProbeArticleCliRequest request) { succeeded.document(), succeeded.text(), request.verbose(), - request.output(), - request.maxChars()); + request.output().orElse(null), + request.maxChars().orElse(null)); case ProbeOutcome.NoContent noContent -> { if (request.verbose()) { writer.writeNoContentDiagnostics(noContent.document()); diff --git a/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCliRequest.java b/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCliRequest.java index fa9ad62..8948db9 100644 --- a/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCliRequest.java +++ b/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCliRequest.java @@ -19,8 +19,8 @@ final class ProbeFeedCliRequest { private final FeedEntrySelection selection; private final Optional xpath; private final boolean verbose; - private final String output; - private final Integer maxChars; + private final Optional output; + private final Optional maxChars; private ProbeFeedCliRequest( URI feedUrl, @@ -28,15 +28,15 @@ private ProbeFeedCliRequest( FeedEntrySelection selection, Optional xpath, boolean verbose, - String output, - Integer maxChars) { + Optional output, + Optional maxChars) { this.feedUrl = Objects.requireNonNull(feedUrl, "feedUrl"); this.method = Objects.requireNonNull(method, "method"); this.selection = Objects.requireNonNull(selection, "selection"); this.xpath = Objects.requireNonNull(xpath, "xpath"); this.verbose = verbose; - this.output = output; - this.maxChars = maxChars; + this.output = Objects.requireNonNull(output, "output"); + this.maxChars = Objects.requireNonNull(maxChars, "maxChars"); } static ProbeFeedCliRequest create( @@ -61,7 +61,13 @@ static ProbeFeedCliRequest create( throw new ParameterException(spec.commandLine(), "--xpath cannot be used with --method feed"); } return new ProbeFeedCliRequest( - validatedUrl, method, selection, normalizedXpath, verbose, output, maxChars); + validatedUrl, + method, + selection, + normalizedXpath, + verbose, + Optional.ofNullable(output), + Optional.ofNullable(maxChars)); } URI feedUrl() { @@ -84,11 +90,11 @@ boolean verbose() { return verbose; } - String output() { + Optional output() { return output; } - Integer maxChars() { + Optional maxChars() { return maxChars; } diff --git a/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCommand.java b/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCommand.java index 9508d14..e46f33d 100644 --- a/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCommand.java +++ b/app/src/main/java/net/sasasin/sreader/cli/ProbeFeedCommand.java @@ -110,8 +110,8 @@ private int mapOutcome(ProbeOutcome outcome, ProbeFeedCliRequest request) { succeeded.document(), succeeded.text(), request.verbose(), - request.output(), - request.maxChars()); + request.output().orElse(null), + request.maxChars().orElse(null)); case ProbeOutcome.NoContent noContent -> { if (request.verbose()) { writer.writeNoContentDiagnostics(noContent.document()); diff --git a/app/src/test/java/net/sasasin/sreader/cli/CliOptionConstraintArchitectureTest.java b/app/src/test/java/net/sasasin/sreader/cli/CliOptionConstraintArchitectureTest.java index 6072a15..135e1d9 100644 --- a/app/src/test/java/net/sasasin/sreader/cli/CliOptionConstraintArchitectureTest.java +++ b/app/src/test/java/net/sasasin/sreader/cli/CliOptionConstraintArchitectureTest.java @@ -79,6 +79,12 @@ void probeArticleUsesTypedRequestFactory() throws IOException { assertThat(mainJavaRoot().resolve("cli/ProbeFeedCliRequest.java")).isRegularFile(); } + @Test + void probeRequestsKeepOptionalOutputAndMaxCharsNonNull() throws IOException { + assertRequestUsesOptionalOutputAndMaxChars("cli/ProbeArticleCliRequest.java"); + assertRequestUsesOptionalOutputAndMaxChars("cli/ProbeFeedCliRequest.java"); + } + @Test void feedImportDoesNotExclusiveGroupDryRunAndResubscribe() throws IOException { String source = readMain("cli/FeedImportCommand.java"); @@ -108,6 +114,15 @@ private static String readMain(String relative) throws IOException { return Files.readString(mainJavaRoot().resolve(relative), StandardCharsets.UTF_8); } + private static void assertRequestUsesOptionalOutputAndMaxChars(String relative) + throws IOException { + String source = readMain(relative); + assertThat(source).contains("private final Optional output;"); + assertThat(source).contains("private final Optional maxChars;"); + assertThat(source).contains("this.output = Objects.requireNonNull(output, \"output\");"); + assertThat(source).contains("this.maxChars = Objects.requireNonNull(maxChars, \"maxChars\");"); + } + private static Path mainJavaRoot() { Path cwd = Path.of("").toAbsolutePath().normalize(); Path fromAppModule = cwd.resolve("src/main/java/net/sasasin/sreader"); diff --git a/app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCliRequestTest.java b/app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCliRequestTest.java index c54cc24..fc01d1f 100644 --- a/app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCliRequestTest.java +++ b/app/src/test/java/net/sasasin/sreader/cli/ProbeArticleCliRequestTest.java @@ -33,6 +33,8 @@ void acceptsArticleCapableMethods(FullTextMethod method) { assertThat(request.url()).isEqualTo(URI.create("https://example.com/a")); assertThat(request.method()).isEqualTo(method); assertThat(request.xpath()).isEmpty(); + assertThat(request.output()).isEmpty(); + assertThat(request.maxChars()).isEmpty(); } @Test @@ -63,8 +65,8 @@ void blankXpathIsAbsence() { assertThat(request.xpath()).isEmpty(); assertThat(request.verbose()).isTrue(); - assertThat(request.output()).isEqualTo("out.txt"); - assertThat(request.maxChars()).isEqualTo(10); + assertThat(request.output()).contains("out.txt"); + assertThat(request.maxChars()).contains(10); } @Test diff --git a/app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCliRequestTest.java b/app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCliRequestTest.java index 6df2bea..6cb5ad8 100644 --- a/app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCliRequestTest.java +++ b/app/src/test/java/net/sasasin/sreader/cli/ProbeFeedCliRequestTest.java @@ -39,6 +39,8 @@ void acceptsAllMethodsWithoutXpath(FullTextMethod method) { assertThat(request.method()).isEqualTo(method); assertThat(request.selection()).isEqualTo(FeedEntrySelection.first()); assertThat(request.xpath()).isEmpty(); + assertThat(request.output()).isEmpty(); + assertThat(request.maxChars()).isEmpty(); } @Test @@ -75,8 +77,8 @@ void acceptsArticleMethodWithXpath() { assertThat(request.xpath()).contains("//article"); assertThat(request.selection()).isEqualTo(FeedEntrySelection.index(1)); assertThat(request.verbose()).isTrue(); - assertThat(request.output()).isEqualTo("out.txt"); - assertThat(request.maxChars()).isEqualTo(20); + assertThat(request.output()).contains("out.txt"); + assertThat(request.maxChars()).contains(20); } @Test