From 6504612acc18a4896273ee8ba3dcac514bb89306 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Thu, 23 Aug 2018 13:03:13 -0500 Subject: [PATCH 1/2] HLRC: Use Optional in validation logic The Validatable class comes from an old class in server code, that assumed null was returned in the event of validation having no errors. This commit changes that to use Optional, which is cleaner than passing around null objects. --- .../client/RestHighLevelClient.java | 13 +++++++------ .../org/elasticsearch/client/Validatable.java | 19 +++++++------------ 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java index 7376f74839ce1..470b7778e0a51 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java @@ -177,6 +177,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.ServiceLoader; import java.util.Set; import java.util.function.Function; @@ -1011,9 +1012,9 @@ protected final Resp performRequest(Req request, RequestOptions options, CheckedFunction responseConverter, Set ignores) throws IOException { - ValidationException validationException = request.validate(); - if (validationException != null && validationException.validationErrors().isEmpty() == false) { - throw validationException; + Optional validationException = request.validate(); + if (validationException != null && validationException.isPresent()) { + throw validationException.get(); } return internalPerformRequest(request, requestConverter, options, responseConverter, ignores); } @@ -1106,9 +1107,9 @@ protected final void performRequestAsync(Req req RequestOptions options, CheckedFunction responseConverter, ActionListener listener, Set ignores) { - ValidationException validationException = request.validate(); - if (validationException != null && validationException.validationErrors().isEmpty() == false) { - listener.onFailure(validationException); + Optional validationException = request.validate(); + if (validationException != null && validationException.isPresent()) { + listener.onFailure(validationException.get()); return; } internalPerformRequestAsync(request, requestConverter, options, responseConverter, listener, ignores); diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/Validatable.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/Validatable.java index 2efff4d3663b8..efb33e795b6b3 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/Validatable.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/Validatable.java @@ -18,24 +18,19 @@ */ package org.elasticsearch.client; +import java.util.Optional; + /** * Defines a validation layer for Requests. */ public interface Validatable { - ValidationException EMPTY_VALIDATION = new ValidationException() { - @Override - public void addValidationError(String error) { - throw new UnsupportedOperationException("Validation messages should not be added to the empty validation"); - } - }; - /** - * Perform validation. This method does not have to be overridden in the event that no validation needs to be done. + * Perform validation. This method does not have to be overridden in the event that no validation needs to be done, + * or the validation was done during object construction time. * - * @return potentially null, in the event of older actions, an empty {@link ValidationException} in newer actions, or finally a - * {@link ValidationException} that contains a list of all failed validation. + * @return An {@link Optional} {@link ValidationException} that may or may not contain a list of validation errors. */ - default ValidationException validate() { - return EMPTY_VALIDATION; + default Optional validate() { + return Optional.empty(); } } From 55d522c054cf5ce88a4cf53997a2e8dfecf2f294 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Mon, 27 Aug 2018 12:14:37 -0500 Subject: [PATCH 2/2] Fix validation wording for clarification --- .../src/main/java/org/elasticsearch/client/Validatable.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/Validatable.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/Validatable.java index efb33e795b6b3..fe4a1fc42cb3b 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/Validatable.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/Validatable.java @@ -26,9 +26,10 @@ public interface Validatable { /** * Perform validation. This method does not have to be overridden in the event that no validation needs to be done, - * or the validation was done during object construction time. + * or the validation was done during object construction time. A {@link ValidationException} that is not null is + * assumed to contain validation errors and will be thrown. * - * @return An {@link Optional} {@link ValidationException} that may or may not contain a list of validation errors. + * @return An {@link Optional} {@link ValidationException} that contains a list of validation errors. */ default Optional validate() { return Optional.empty();