Skip to content

Commit

Permalink
[Transform] Report transforms without config as erroneous. (#81141)
Browse files Browse the repository at this point in the history
  • Loading branch information
@przemekwitek
przemekwitek committed Dec 13, 2021
1 parent 7d1c434 commit 2815a77
Show file tree
Hide file tree
Showing 17 changed files with 696 additions and 177 deletions.
80 changes: 72 additions & 8 deletions .../core/src/main/java/org/elasticsearch/xpack/core/transform/action/GetTransformAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

package org.elasticsearch.xpack.core.transform.action;

import org.elasticsearch.Version;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.ActionType;
import org.elasticsearch.common.ValidationException;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
Expand All @@ -27,6 +29,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

import static org.elasticsearch.action.ValidateActions.addValidationError;

Expand Down Expand Up @@ -79,31 +82,77 @@ public String getResourceIdField() {
}
}

public static class Response extends AbstractGetResourcesResponse<TransformConfig> implements Writeable, ToXContentObject {
public static class Response extends AbstractGetResourcesResponse<TransformConfig> implements ToXContentObject {

public static class Error implements Writeable, ToXContentObject {
private static final ParseField TYPE = new ParseField("type");
private static final ParseField REASON = new ParseField("reason");

private final String type;
private final String reason;

public Error(String type, String reason) {
this.type = Objects.requireNonNull(type);
this.reason = Objects.requireNonNull(reason);
}

public Error(StreamInput in) throws IOException {
this.type = in.readString();
this.reason = in.readString();
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(TYPE.getPreferredName(), type);
builder.field(REASON.getPreferredName(), reason);
builder.endObject();
return builder;
}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(type);
out.writeString(reason);
}
}

public static final String INVALID_TRANSFORMS_DEPRECATION_WARNING = "Found [{}] invalid transforms";
private static final ParseField INVALID_TRANSFORMS = new ParseField("invalid_transforms");
private static final ParseField ERRORS = new ParseField("errors");

public Response(List<TransformConfig> transformConfigs, long count) {
super(new QueryPage<>(transformConfigs, count, TransformField.TRANSFORMS));
}
private final List<Error> errors;

public Response() {
super();
public Response(List<TransformConfig> transformConfigs, long count, List<Error> errors) {
super(new QueryPage<>(transformConfigs, count, TransformField.TRANSFORMS));
this.errors = errors;
}

public Response(StreamInput in) throws IOException {
super(in);
if (in.getVersion().onOrAfter(Version.V_8_1_0)) {
if (in.readBoolean()) {
this.errors = in.readList(Error::new);
} else {
this.errors = null;
}
} else {
this.errors = null;
}
}

public List<TransformConfig> getTransformConfigurations() {
return getResources().results();
}

public long getCount() {
public long getTransformConfigurationCount() {
return getResources().count();
}

public List<Error> getErrors() {
return errors;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
List<String> invalidTransforms = new ArrayList<>();
Expand Down Expand Up @@ -132,11 +181,26 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
invalidTransforms.size()
);
}

if (errors != null) {
builder.field(ERRORS.getPreferredName(), errors);
}
builder.endObject();
return builder;
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
if (out.getVersion().onOrAfter(Version.V_8_1_0)) {
if (errors != null) {
out.writeBoolean(true);
out.writeList(errors);
} else {
out.writeBoolean(false);
}
}
}

@Override
protected Reader<TransformConfig> getReader() {
return TransformConfig::new;
Expand Down
105 changes: 80 additions & 25 deletions ...t/java/org/elasticsearch/xpack/core/transform/action/GetTransformActionResponseTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,52 +19,107 @@
import org.elasticsearch.xpack.core.watcher.watch.Payload.XContent;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;

public class GetTransformActionResponseTests extends AbstractWireSerializingTransformTestCase<Response> {

public static Response randomTransformResponse() {
List<TransformConfig> configs = new ArrayList<>();
int totalConfigs = randomInt(10);
for (int i = 0; i < totalConfigs; ++i) {
configs.add(TransformConfigTests.randomTransformConfig());
}
List<TransformConfig> configs = randomList(0, 10, () -> TransformConfigTests.randomTransformConfig());
List<Response.Error> errors = randomBoolean() ? randomList(1, 5, () -> randomError()) : null;
return new Response(configs, randomNonNegativeLong(), errors);
}

return new Response(configs, randomNonNegativeLong());
private static Response.Error randomError() {
return new Response.Error(randomAlphaOfLengthBetween(1, 10), randomAlphaOfLengthBetween(1, 20));
}

public void testInvalidTransforms() throws IOException {
List<TransformConfig> transforms = new ArrayList<>();

transforms.add(TransformConfigTests.randomTransformConfig());
transforms.add(TransformConfigTests.randomInvalidTransformConfig());
transforms.add(TransformConfigTests.randomTransformConfig());
transforms.add(TransformConfigTests.randomInvalidTransformConfig());
List<TransformConfig> transforms = List.of(
TransformConfigTests.randomTransformConfig("valid-transform-1"),
TransformConfigTests.randomInvalidTransformConfig("invalid-transform-1"),
TransformConfigTests.randomTransformConfig("valid-transform-2"),
TransformConfigTests.randomInvalidTransformConfig("invalid-transform-2")
);

Response r = new Response(transforms, transforms.size());
Response r = new Response(transforms, transforms.size(), null);
XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
r.toXContent(builder, XContent.EMPTY_PARAMS);
Map<String, Object> responseAsMap = createParser(builder).map();
assertEquals(2, XContentMapValues.extractValue("invalid_transforms.count", responseAsMap));
List<String> expectedInvalidTransforms = new ArrayList<>();
expectedInvalidTransforms.add(transforms.get(1).getId());
expectedInvalidTransforms.add(transforms.get(3).getId());
List<String> expectedInvalidTransforms = List.of("invalid-transform-1", "invalid-transform-2");
assertEquals(expectedInvalidTransforms, XContentMapValues.extractValue("invalid_transforms.transforms", responseAsMap));
assertWarnings(LoggerMessageFormat.format(Response.INVALID_TRANSFORMS_DEPRECATION_WARNING, 2));
}

public void testErrors() throws IOException {
List<Response.Error> errors = List.of(
new Response.Error("error-type-1", "error-message-1"),
new Response.Error("error-type-2", "error-message-2"),
new Response.Error("error-type-3", "error-message-3")
);

Response r = new Response(List.of(), 0, errors);
XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
r.toXContent(builder, XContent.EMPTY_PARAMS);
Map<String, Object> responseAsMap = createParser(builder).map();
assertThat(XContentMapValues.extractValue("invalid_transforms", responseAsMap), is(nullValue()));
assertThat(
XContentMapValues.extractValue("errors", responseAsMap),
is(
equalTo(
List.of(
Map.of("type", "error-type-1", "reason", "error-message-1"),
Map.of("type", "error-type-2", "reason", "error-message-2"),
Map.of("type", "error-type-3", "reason", "error-message-3")
)
)
)
);
ensureNoWarnings();
}

public void testBothInvalidConfigsAndErrors() throws IOException {
List<TransformConfig> transforms = List.of(TransformConfigTests.randomInvalidTransformConfig("invalid-transform-7"));
List<Response.Error> errors = List.of(
new Response.Error("error-type-1", "error-message-1"),
new Response.Error("error-type-2", "error-message-2"),
new Response.Error("error-type-3", "error-message-3")
);

Response r = new Response(transforms, transforms.size(), errors);
XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
r.toXContent(builder, XContent.EMPTY_PARAMS);
Map<String, Object> responseAsMap = createParser(builder).map();
assertThat(XContentMapValues.extractValue("invalid_transforms.count", responseAsMap), is(equalTo(1)));
assertThat(
XContentMapValues.extractValue("invalid_transforms.transforms", responseAsMap),
is(equalTo(List.of("invalid-transform-7")))
);
assertThat(
XContentMapValues.extractValue("errors", responseAsMap),
is(
equalTo(
List.of(
Map.of("type", "error-type-1", "reason", "error-message-1"),
Map.of("type", "error-type-2", "reason", "error-message-2"),
Map.of("type", "error-type-3", "reason", "error-message-3")
)
)
)
);
assertWarnings(LoggerMessageFormat.format(Response.INVALID_TRANSFORMS_DEPRECATION_WARNING, 1));
}

@SuppressWarnings("unchecked")
public void testNoHeaderInResponse() throws IOException {
List<TransformConfig> transforms = new ArrayList<>();
int totalConfigs = randomInt(10);

for (int i = 0; i < totalConfigs; ++i) {
transforms.add(TransformConfigTests.randomTransformConfig());
}
List<TransformConfig> transforms = randomList(0, 10, () -> TransformConfigTests.randomTransformConfig());

Response r = new Response(transforms, transforms.size());
Response r = new Response(transforms, transforms.size(), null);
XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
r.toXContent(builder, XContent.EMPTY_PARAMS);
Map<String, Object> responseAsMap = createParser(builder).map();
Expand All @@ -79,7 +134,7 @@ public void testNoHeaderInResponse() throws IOException {
for (int i = 0; i < transforms.size(); ++i) {
assertArrayEquals(
transforms.get(i).getSource().getIndex(),
((ArrayList<String>) XContentMapValues.extractValue("source.index", transformsResponse.get(i))).toArray(new String[0])
((List<String>) XContentMapValues.extractValue("source.index", transformsResponse.get(i))).toArray(new String[0])
);
assertEquals(null, XContentMapValues.extractValue("headers", transformsResponse.get(i)));
}
Expand Down
6 changes: 3 additions & 3 deletions ...src/test/java/org/elasticsearch/xpack/core/transform/transforms/TransformConfigTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public static TransformConfig randomTransformConfig(String id, Version version,
);
}

public static TransformConfig randomInvalidTransformConfig() {
public static TransformConfig randomInvalidTransformConfig(String id) {
if (randomBoolean()) {
PivotConfig pivotConfig;
LatestConfig latestConfig;
Expand All @@ -154,7 +154,7 @@ public static TransformConfig randomInvalidTransformConfig() {
}

return new TransformConfig(
randomAlphaOfLengthBetween(1, 10),
id,
randomInvalidSourceConfig(),
randomDestConfig(),
null,
Expand All @@ -171,7 +171,7 @@ public static TransformConfig randomInvalidTransformConfig() {
);
} // else
return new TransformConfig(
randomAlphaOfLengthBetween(1, 10),
id,
randomSourceConfig(),
randomDestConfig(),
null,
Expand Down
2 changes: 1 addition & 1 deletion ...cation/src/main/java/org/elasticsearch/xpack/deprecation/TransformDeprecationChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private void recursiveGetTransformsAndCollectDeprecations(
for (TransformConfig config : getTransformResponse.getTransformConfigurations()) {
issues.addAll(config.checkForDeprecations(components.xContentRegistry()));
}
if (getTransformResponse.getCount() >= (page.getFrom() + page.getSize())) {
if (getTransformResponse.getTransformConfigurationCount() >= (page.getFrom() + page.getSize())) {
PageParams nextPage = new PageParams(page.getFrom() + page.getSize(), PageParams.DEFAULT_SIZE);
recursiveGetTransformsAndCollectDeprecations(components, issues, nextPage, listener);
} else {
Expand Down

0 comments on commit 2815a77

Please sign in to comment.