Skip to content

Commit 2d59c84

Browse files
dimitris-athanasioudimitris-athanasiou
authored
[ML] Reuse SourceDestValidator for data frame analytics (#50841)
This commit removes validation logic of source and dest indices for data frame analytics and replaces it with using the common `SourceDestValidator` class which is already used by transforms. This way the validations and their messages become consistent while we reduce code. This means that where these validations fail the error messages will be slightly different for data frame analytics.
1 parent ce50e8e commit 2d59c84

File tree

18 files changed

+256
-385
lines changed

18 files changed

+256
-385
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/common/validation/SourceDestValidator.java‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ public final class SourceDestValidator {
4646

4747
// messages
4848
public static final String SOURCE_INDEX_MISSING = "Source index [{0}] does not exist";
49-
public static final String SOURCE_LOWERCASE = "Source index [{0}] must be lowercase";
5049
public static final String DEST_IN_SOURCE = "Destination index [{0}] is included in source expression [{1}]";
5150
public static final String DEST_LOWERCASE = "Destination index [{0}] must be lowercase";
5251
public static final String NEEDS_REMOTE_CLUSTER_SEARCH = "Source index is configured with a remote index pattern(s) [{0}]"
@@ -59,6 +58,7 @@ public final class SourceDestValidator {
5958
+ "alias [{0}], at least a [{1}] license is required, found license [{2}]";
6059
public static final String REMOTE_CLUSTER_LICENSE_INACTIVE = "License check failed for remote cluster "
6160
+ "alias [{0}], license is not active";
61+
public static final String REMOTE_SOURCE_INDICES_NOT_SUPPORTED = "remote source indices are not supported";
6262

6363
private final IndexNameExpressionResolver indexNameExpressionResolver;
6464
private final RemoteClusterService remoteClusterService;
@@ -216,7 +216,7 @@ private void resolveLocalAndRemoteSource() {
216216
}
217217
}
218218

219-
interface SourceDestValidation {
219+
public interface SourceDestValidation {
220220
void validate(Context context, ActionListener<Context> listener);
221221
}
222222

@@ -228,18 +228,7 @@ interface SourceDestValidation {
228228
public static final SourceDestValidation REMOTE_SOURCE_VALIDATION = new RemoteSourceEnabledAndRemoteLicenseValidation();
229229
public static final SourceDestValidation DESTINATION_IN_SOURCE_VALIDATION = new DestinationInSourceValidation();
230230
public static final SourceDestValidation DESTINATION_SINGLE_INDEX_VALIDATION = new DestinationSingleIndexValidation();
231-
232-
// set of default validation collections, if you want to automatically benefit from new validators, use those
233-
public static final List<SourceDestValidation> PREVIEW_VALIDATIONS = Arrays.asList(SOURCE_MISSING_VALIDATION, REMOTE_SOURCE_VALIDATION);
234-
235-
public static final List<SourceDestValidation> ALL_VALIDATIONS = Arrays.asList(
236-
SOURCE_MISSING_VALIDATION,
237-
REMOTE_SOURCE_VALIDATION,
238-
DESTINATION_IN_SOURCE_VALIDATION,
239-
DESTINATION_SINGLE_INDEX_VALIDATION
240-
);
241-
242-
public static final List<SourceDestValidation> NON_DEFERABLE_VALIDATIONS = Arrays.asList(DESTINATION_SINGLE_INDEX_VALIDATION);
231+
public static final SourceDestValidation REMOTE_SOURCE_NOT_SUPPORTED_VALIDATION = new RemoteSourceNotSupportedValidation();
243232

244233
/**
245234
* Create a new Source Dest Validator
@@ -299,10 +288,11 @@ public void validate(
299288
}
300289
}, listener::onFailure);
301290

291+
// We traverse the validations in reverse order as we chain the listeners from back to front
302292
for (int i = validations.size() - 1; i >= 0; i--) {
303-
final SourceDestValidation validation = validations.get(i);
293+
SourceDestValidation validation = validations.get(i);
304294
final ActionListener<Context> previousValidationListener = validationListener;
305-
validationListener = ActionListener.wrap(c -> { validation.validate(c, previousValidationListener); }, listener::onFailure);
295+
validationListener = ActionListener.wrap(c -> validation.validate(c, previousValidationListener), listener::onFailure);
306296
}
307297

308298
validationListener.onResponse(context);
@@ -427,13 +417,13 @@ public void validate(Context context, ActionListener<Context> listener) {
427417
return;
428418
}
429419

430-
if (context.resolvedSource.contains(destIndex)) {
420+
if (context.resolveSource().contains(destIndex)) {
431421
context.addValidationError(DEST_IN_SOURCE, destIndex, Strings.arrayToCommaDelimitedString(context.getSource()));
432422
listener.onResponse(context);
433423
return;
434424
}
435425

436-
if (context.resolvedSource.contains(context.resolveDest())) {
426+
if (context.resolveSource().contains(context.resolveDest())) {
437427
context.addValidationError(
438428
DEST_IN_SOURCE,
439429
context.resolveDest(),
@@ -454,6 +444,17 @@ public void validate(Context context, ActionListener<Context> listener) {
454444
}
455445
}
456446

447+
static class RemoteSourceNotSupportedValidation implements SourceDestValidation {
448+
449+
@Override
450+
public void validate(Context context, ActionListener<Context> listener) {
451+
if (context.resolveRemoteSource().isEmpty() == false) {
452+
context.addValidationError(REMOTE_SOURCE_INDICES_NOT_SUPPORTED);
453+
}
454+
listener.onResponse(context);
455+
}
456+
}
457+
457458
private static String getMessage(String message, Object... args) {
458459
return new MessageFormat(message, Locale.ROOT).format(args);
459460
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutDataFrameAnalyticsAction.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@
1818
import org.elasticsearch.common.xcontent.ToXContentObject;
1919
import org.elasticsearch.common.xcontent.XContentBuilder;
2020
import org.elasticsearch.common.xcontent.XContentParser;
21+
import org.elasticsearch.xpack.core.common.validation.SourceDestValidator;
2122
import org.elasticsearch.xpack.core.ml.dataframe.DataFrameAnalyticsConfig;
2223
import org.elasticsearch.xpack.core.ml.dataframe.DataFrameAnalyticsSource;
2324
import org.elasticsearch.xpack.core.ml.job.messages.Messages;
25+
import org.elasticsearch.xpack.core.ml.utils.MlStrings;
2426

2527
import java.io.IOException;
2628
import java.util.Objects;
@@ -90,14 +92,29 @@ public DataFrameAnalyticsConfig getConfig() {
9092
@Override
9193
public ActionRequestValidationException validate() {
9294
ActionRequestValidationException error = null;
95+
error = checkConfigIdIsValid(config, error);
96+
error = SourceDestValidator.validateRequest(error, config.getDest().getIndex());
9397
error = checkNoIncludedAnalyzedFieldsAreExcludedBySourceFiltering(config, error);
9498
return error;
9599
}
96100

101+
private ActionRequestValidationException checkConfigIdIsValid(DataFrameAnalyticsConfig config,
102+
ActionRequestValidationException error) {
103+
if (MlStrings.isValidId(config.getId()) == false) {
104+
error = ValidateActions.addValidationError(Messages.getMessage(Messages.INVALID_ID, DataFrameAnalyticsConfig.ID,
105+
config.getId()), error);
106+
}
107+
if (!MlStrings.hasValidLengthForId(config.getId())) {
108+
error = ValidateActions.addValidationError(Messages.getMessage(Messages.ID_TOO_LONG, DataFrameAnalyticsConfig.ID,
109+
config.getId(), MlStrings.ID_LENGTH_LIMIT), error);
110+
}
111+
return error;
112+
}
113+
97114
private ActionRequestValidationException checkNoIncludedAnalyzedFieldsAreExcludedBySourceFiltering(
98115
DataFrameAnalyticsConfig config, ActionRequestValidationException error) {
99116
if (config.getAnalyzedFields() == null) {
100-
return null;
117+
return error;
101118
}
102119
for (String analyzedInclude : config.getAnalyzedFields().includes()) {
103120
if (config.getSource().isFieldExcluded(analyzedInclude)) {
@@ -107,7 +124,7 @@ private ActionRequestValidationException checkNoIncludedAnalyzedFieldsAreExclude
107124
+ DataFrameAnalyticsSource._SOURCE.getPreferredName() + "]", error);
108125
}
109126
}
110-
return null;
127+
return error;
111128
}
112129

113130
@Override

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/DataFrameAnalyticsDest.java‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,11 @@
1313
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
1414
import org.elasticsearch.common.xcontent.ToXContentObject;
1515
import org.elasticsearch.common.xcontent.XContentBuilder;
16-
import org.elasticsearch.indices.InvalidIndexNameException;
1716
import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
1817

1918
import java.io.IOException;
20-
import java.util.Locale;
2119
import java.util.Objects;
2220

23-
import static org.elasticsearch.cluster.metadata.MetaDataCreateIndexService.validateIndexOrAliasName;
24-
2521
public class DataFrameAnalyticsDest implements Writeable, ToXContentObject {
2622

2723
public static final ParseField INDEX = new ParseField("index");
@@ -94,13 +90,4 @@ public String getIndex() {
9490
public String getResultsField() {
9591
return resultsField;
9692
}
97-
98-
public void validate() {
99-
if (index != null) {
100-
validateIndexOrAliasName(index, InvalidIndexNameException::new);
101-
if (index.toLowerCase(Locale.ROOT).equals(index) == false) {
102-
throw new InvalidIndexNameException(index, "dest.index must be lowercase");
103-
}
104-
}
105-
}
10693
}

0 commit comments

Comments
 (0)