diff options
4 files changed, 27 insertions, 24 deletions
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java index 0485689b15d..1224e668bc0 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java @@ -35,6 +35,7 @@ import java.time.Instant; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; /** * Sends operations to messagebus via document api. @@ -352,12 +353,16 @@ public class OperationHandlerImpl implements OperationHandler { protected static ClusterDef resolveClusterDef(Optional<String> wantedCluster, List<ClusterDef> clusters) throws RestApiException { if (clusters.size() == 0) { throw new IllegalArgumentException("Your Vespa cluster does not have any content clusters " + - "declared. Visiting feature is not available."); + "declared. Visiting feature is not available."); } if (! wantedCluster.isPresent()) { if (clusters.size() != 1) { - throw new RestApiException(Response.createErrorResponse(400, "Several clusters exist: " + - clusterListToString(clusters) + " you must specify one. ", RestUri.apiErrorCodes.SEVERAL_CLUSTERS)); + String message = "Several clusters exist: " + + clusters.stream().map(c -> "'" + c.getName() + "'").collect(Collectors.joining(", ")) + + ". You must specify one."; + throw new RestApiException(Response.createErrorResponse(400, + message, + RestUri.apiErrorCodes.SEVERAL_CLUSTERS)); } return clusters.get(0); } @@ -367,20 +372,18 @@ public class OperationHandlerImpl implements OperationHandler { return clusterDef; } } - throw new RestApiException(Response.createErrorResponse(400, "Your vespa cluster contains the content clusters " + - clusterListToString(clusters) + " not " + wantedCluster.get() + ". Please select a valid vespa cluster.", RestUri.apiErrorCodes.MISSING_CLUSTER)); + String message = "Your vespa cluster contains the content clusters " + + clusters.stream().map(c -> "'" + c.getName() + "'").collect(Collectors.joining(", ")) + + ", not '" + wantedCluster.get() + "'. Please select a valid vespa cluster."; + throw new RestApiException(Response.createErrorResponse(400, + message, + RestUri.apiErrorCodes.MISSING_CLUSTER)); } protected static String clusterDefToRoute(ClusterDef clusterDef) { return "[Storage:cluster=" + clusterDef.getName() + ";clusterconfigid=" + clusterDef.getConfigId() + "]"; } - private static String clusterListToString(List<ClusterDef> clusters) { - StringBuilder clusterListString = new StringBuilder(); - clusters.forEach(x -> clusterListString.append(x.getName()).append(" (").append(x.getConfigId()).append("), ")); - return clusterListString.toString(); - } - private static String buildAugmentedDocumentSelection(RestUri restUri, String documentSelection) { if (restUri.isRootOnly()) { return documentSelection; // May be empty, that's fine. diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java index 4e5092cd416..bda49ecd3f5 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java @@ -94,8 +94,8 @@ public class OperationHandlerImplTest { assertThat(e.getResponse().getStatus(), is(400)); String errorMsg = renderRestApiExceptionAsString(e); assertThat(errorMsg, is("{\"errors\":[{\"description\":" + - "\"MISSING_CLUSTER Your vespa cluster contains the content clusters foo2 (configId2), foo (configId)," + - " foo3 (configId2), not wrong. Please select a valid vespa cluster.\",\"id\":-9}]}")); + "\"MISSING_CLUSTER Your vespa cluster contains the content clusters 'foo2', 'foo'," + + " 'foo3', not 'wrong'. Please select a valid vespa cluster.\",\"id\":-9}]}")); return; } fail("Expected exception"); diff --git a/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java b/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java index 307cf979c33..83199c76e5a 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java @@ -25,6 +25,7 @@ import org.apache.commons.cli.Options; import java.io.*; import java.nio.charset.Charset; import java.util.Map; +import java.util.stream.Collectors; /** * Client using visiting, used by the vespa-visit command line tool. @@ -572,18 +573,14 @@ public class VdsVisit { protected static String resolveClusterRoute(ClusterList clusters, String wantedCluster) { if (clusters.getStorageClusters().size() == 0) { throw new IllegalArgumentException("Your Vespa cluster does not have any content clusters " + - "declared. Visiting feature is not available."); + "declared. Visiting feature is not available."); } ClusterDef found = null; - String names = ""; - for (ClusterDef c : clusters.getStorageClusters()) { - if (!names.isEmpty()) { - names += ", "; - } - names += c.getName(); - } + String names = clusters.getStorageClusters() + .stream().map(c -> "'" + c.getName() + "'") + .collect(Collectors.joining(", ")); if (wantedCluster != null) { for (ClusterDef c : clusters.getStorageClusters()) { if (c.getName().equals(wantedCluster)) { @@ -592,13 +589,14 @@ public class VdsVisit { } if (found == null) { throw new IllegalArgumentException("Your vespa cluster contains the content clusters " + - names + ", not " + wantedCluster + ". Please select a valid vespa cluster."); + names + ", not '" + wantedCluster + + "'. Please select a valid vespa cluster."); } } else if (clusters.getStorageClusters().size() == 1) { found = clusters.getStorageClusters().get(0); } else { throw new IllegalArgumentException("Your vespa cluster contains the content clusters " + - names + ". Please use the -c option to select one of them as a target for visiting."); + names + ". Please use the -c option to select one of them as a target for visiting."); } return "[Storage:cluster=" + found.getName() + ";clusterconfigid=" + found.getConfigId() + "]"; diff --git a/vespaclient-java/src/test/java/com/yahoo/vespavisit/VdsVisitTestCase.java b/vespaclient-java/src/test/java/com/yahoo/vespavisit/VdsVisitTestCase.java index 20f3cbcfe23..d9efe2c5129 100644 --- a/vespaclient-java/src/test/java/com/yahoo/vespavisit/VdsVisitTestCase.java +++ b/vespaclient-java/src/test/java/com/yahoo/vespavisit/VdsVisitTestCase.java @@ -242,7 +242,9 @@ public class VdsVisitTestCase { try { VdsVisit.resolveClusterRoute(clusterList, "borkbork"); } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("Your vespa cluster contains the content clusters storage, not borkbork.")); + assertEquals("Your vespa cluster contains the content clusters 'storage', not 'borkbork'. " + + "Please select a valid vespa cluster.", + e.getMessage()); } } |