diff options
4 files changed, 52 insertions, 67 deletions
diff --git a/messagebus/src/main/java/com/yahoo/messagebus/SourceSession.java b/messagebus/src/main/java/com/yahoo/messagebus/SourceSession.java index 920d285d200..f4aed92e8ce 100644 --- a/messagebus/src/main/java/com/yahoo/messagebus/SourceSession.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/SourceSession.java @@ -29,14 +29,14 @@ public final class SourceSession implements ReplyHandler, MessageBus.SendBlocked private final Queue<BlockedMessage> blockedQ = new LinkedList<>(); /** - * <p>The default constructor requires values for all final member variables + * The default constructor requires values for all final member variables * of this. It expects all arguments but the {@link SourceSessionParams} to * be proper, so no checks are performed. The constructor is declared * package private since only {@link MessageBus} is supposed to instantiate - * it.</p> + * it. * - * @param mbus The message bus that created this instance. - * @param params A parameter object that holds configuration parameters. + * @param mbus the message bus that created this instance + * @param params a parameter object that holds configuration parameters */ SourceSession(MessageBus mbus, SourceSessionParams params) { this.mbus = mbus; @@ -221,15 +221,15 @@ public final class SourceSession implements ReplyHandler, MessageBus.SendBlocked } /** - * <p>This is a blocking proxy to the {@link #send(Message)} method. This + * This is a blocking proxy to the {@link #send(Message)} method. This * method blocks until the message is accepted by the send queue. Note that * the message timeout does not activate by calling this method. This method * will also return if this session is closed or the calling thread is - * interrupted.</p> + * interrupted. * - * @param msg The message to send. - * @return The result of initiating send. - * @throws InterruptedException Thrown if the calling thread is interrupted. + * @param msg the message to send + * @return the result of initiating send + * @throws InterruptedException thrown if the calling thread is interrupted */ public Result sendBlocking(Message msg) throws InterruptedException { Result res = send(msg); @@ -287,44 +287,43 @@ public final class SourceSession implements ReplyHandler, MessageBus.SendBlocked } /** - * <p>This is a convenience function to assign a given route to the given + * This is a convenience function to assign a given route to the given * message, and then pass it to the other {@link #send(Message)} method of - * this session.</p> + * this session. * - * @param msg The message to send. - * @param route The route to assign to the message. - * @return The immediate result of the attempt to send this message. + * @param msg the message to send + * @param route the route to assign to the message + * @return the immediate result of the attempt to send this message */ public Result send(Message msg, Route route) { return send(msg.setRoute(route)); } /** - * <p>This is a convenience method to call {@link + * This is a convenience method to call {@link * #send(Message,String,boolean)} with a <code>false</code> value for the - * 'parseIfNotFound' parameter.</p> + * 'parseIfNotFound' parameter. * - * @param msg The message to send. - * @param routeName The route to assign to the message. - * @return The immediate result of the attempt to send this message. + * @param msg the message to send + * @param routeName the route to assign to the message + * @return the immediate result of the attempt to send this message */ public Result send(Message msg, String routeName) { return send(msg, routeName, false); } /** - * <p>This is a convenience function to assign a named route to the given + * This is a convenience function to assign a named route to the given * message, and then pass it to the other {@link #send(Message)} method of * this session. If the route could not be found this methods returns with * an appropriate error, unless the 'parseIfNotFound' argument is true. In * that case, the route name is passed through to the Route factory method - * {@link Route#parse}.</p> + * {@link Route#parse}. * - * @param msg The message to send. - * @param routeName The route to assign to the message. - * @param parseIfNotFound Whether or not to parse routeName as a route if - * it could not be found. - * @return The immediate result of the attempt to send this message. + * @param msg the message to send + * @param routeName the route to assign to the message + * @param parseIfNotFound whether or not to parse routeName as a route if it could not be found + * @return the immediate result of the attempt to send this message */ public Result send(Message msg, String routeName, boolean parseIfNotFound) { boolean found = false; @@ -334,48 +333,39 @@ public final class SourceSession implements ReplyHandler, MessageBus.SendBlocked if (route != null) { msg.setRoute(new Route(route)); found = true; - } else if (!parseIfNotFound) { + } else if ( ! parseIfNotFound) { return new Result(ErrorCode.ILLEGAL_ROUTE, "Route '" + routeName + "' not found for protocol '" + msg.getProtocol() + "'."); } - } else if (!parseIfNotFound) { + } else if ( ! parseIfNotFound) { return new Result(ErrorCode.ILLEGAL_ROUTE, "Protocol '" + msg.getProtocol() + "' has no routing table."); } - if (!found) { + if ( ! found) { msg.setRoute(Route.parse(routeName)); } return send(msg); } - /** - * <p>Returns the reply handler of this session.</p> - * - * @return The reply handler. - */ + /** Returns the reply handler of this session */ public ReplyHandler getReplyHandler() { return replyHandler; } - /** - * <p>Returns the number of messages sent that have not been replied to - * yet.</p> - * - * @return The pending count. - */ + /** Returns the number of messages sent that have not been replied to yet */ public int getPendingCount() { return pendingCount; } /** - * <p>Sets the number of seconds a message can be attempted sent until it - * times out.</p> + * Sets the number of seconds a message can be attempted sent until it times out. * - * @param timeout The numer of seconds allowed. - * @return This, to allow chaining. + * @param timeout the number of seconds allowed. + * @return this, to allow chaining. */ public SourceSession setTimeout(double timeout) { this.timeout = timeout; return this; } + } diff --git a/messagebus/src/main/java/com/yahoo/messagebus/routing/Route.java b/messagebus/src/main/java/com/yahoo/messagebus/routing/Route.java index 9190b680ebf..4a5ba03a301 100755 --- a/messagebus/src/main/java/com/yahoo/messagebus/routing/Route.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/routing/Route.java @@ -47,11 +47,11 @@ public class Route { } /** - * <p>Parses the given string as a list of space-separated hops. The {@link - * #toString()} method is compatible with this parser.</p> + * Parses the given string as a list of space-separated hops. The {@link #toString()} + * method is compatible with this parser. * - * @param str The string to parse. - * @return A route that corresponds to the string. + * @param str the string to parse + * @return a route that corresponds to the string */ public static Route parse(String str) { if (str == null || str.length() == 0) { 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 cf22d25b510..0f4adf0179f 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 @@ -21,12 +21,12 @@ import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol; import com.yahoo.documentapi.metrics.DocumentApiMetrics; import com.yahoo.documentapi.metrics.DocumentOperationStatus; import com.yahoo.documentapi.metrics.DocumentOperationType; -import com.yahoo.messagebus.ErrorCode; import com.yahoo.messagebus.StaticThrottlePolicy; import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.vdslib.VisitorOrdering; import com.yahoo.vespaclient.ClusterDef; import com.yahoo.vespaxmlparser.FeedOperation; +import com.yahoo.vespaxmlparser.VespaXMLFeedReader; import com.yahoo.yolean.concurrent.ConcurrentResourcePool; import com.yahoo.yolean.concurrent.ResourceFactory; import org.apache.commons.lang3.exception.ExceptionUtils; @@ -108,19 +108,10 @@ public class OperationHandlerImpl implements OperationHandler { documentAccess.shutdown(); } - private static final int HTTP_STATUS_SERVER_ERROR = 500; private static final int HTTP_STATUS_BAD_REQUEST = 400; private static final int HTTP_STATUS_INSUFFICIENT_STORAGE = 507; private static final int HTTP_PRE_CONDIDTION_FAILED = 412; - private static boolean isFatalMBusErrorCode(Set<Integer> errorCodes) { - for (Integer e : errorCodes) { - if (ErrorCode.isFatal(e) && ErrorCode.isMBusError(e)) { - return true; - } - } - return false; - } public static int getHTTPStatusCode(Set<Integer> errorCodes) { if (errorCodes.size() == 1 && errorCodes.contains(DocumentProtocol.ERROR_NO_SPACE)) { return HTTP_STATUS_INSUFFICIENT_STORAGE; @@ -128,9 +119,6 @@ public class OperationHandlerImpl implements OperationHandler { if (errorCodes.contains(DocumentProtocol.ERROR_TEST_AND_SET_CONDITION_FAILED)) { return HTTP_PRE_CONDIDTION_FAILED; } - if (isFatalMBusErrorCode(errorCodes)) { - return HTTP_STATUS_SERVER_ERROR; - } return HTTP_STATUS_BAD_REQUEST; } @@ -182,10 +170,9 @@ public class OperationHandlerImpl implements OperationHandler { } } - private VisitResult doVisit( - VisitorControlHandler visitorControlHandler, - LocalDataVisitorHandler localDataVisitorHandler, - RestUri restUri) throws RestApiException { + private VisitResult doVisit(VisitorControlHandler visitorControlHandler, + LocalDataVisitorHandler localDataVisitorHandler, + RestUri restUri) throws RestApiException { try { visitorControlHandler.waitUntilDone(); // VisitorParameters' session timeout implicitly triggers timeout failures. throwIfFatalVisitingError(visitorControlHandler, restUri); @@ -208,7 +195,8 @@ public class OperationHandlerImpl implements OperationHandler { if (! (session instanceof MessageBusSyncSession)) { // Not sure if this ever could happen but better be safe. throw new RestApiException(Response.createErrorResponse( - 400, "Can not set route since the API is not using message bus.", RestUri.apiErrorCodes.NO_ROUTE_WHEN_NOT_PART_OF_MESSAGEBUS)); + 400, "Can not set route since the API is not using message bus.", + RestUri.apiErrorCodes.NO_ROUTE_WHEN_NOT_PART_OF_MESSAGEBUS)); } ((MessageBusSyncSession) session).setRoute(route.orElse("default")); } diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java index b4992fb519b..698c37244a1 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java @@ -210,7 +210,7 @@ public class RestApi extends LoggingRequestHandler { } String condition = request.getProperty(CONDITION_PARAMETER_NAME); - Optional<String> route = Optional.ofNullable(request.getProperty(ROUTE_PARAMETER_NAME)); + Optional<String> route = Optional.ofNullable(nonEmpty(request.getProperty(ROUTE_PARAMETER_NAME), ROUTE_PARAMETER_NAME)); Optional<ObjectNode> resultJson = Optional.empty(); switch (request.getMethod()) { @@ -351,6 +351,13 @@ public class RestApi extends LoggingRequestHandler { return builder.toString(); } + + private String nonEmpty(String value, String name) { + if (value != null && value.isEmpty()) + throw new IllegalArgumentException("'" + name + "' cannot be empty"); + return value; + } + private static long parseAndValidateVisitNumericId(String value) { try { return Long.parseLong(value); |