Skip to content

Commit f0a1703

Browse files
Merge branch 'main' into sa-failure-store-cleanup-def-method
2 parents 3982010 + 83300ea commit f0a1703

File tree

8 files changed

+140
-45
lines changed

8 files changed

+140
-45
lines changed

BUILDING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ To wire this registered cluster into a `TestClusterAware` task (e.g. `RestIntegT
144144
Additional integration tests for a certain Elasticsearch modules that are specific to certain cluster configuration can be declared in a separate so called `qa` subproject of your module.
145145

146146
The benefit of a dedicated project for these tests are:
147-
- `qa` projects are dedicated two specific use-cases and easier to maintain
147+
- `qa` projects are dedicated to specific use-cases and easier to maintain
148148
- It keeps the specific test logic separated from the common test logic.
149149
- You can run those tests in parallel to other projects of the build.
150150

docs/changelog/127229.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 127229
2+
summary: Return BAD_REQUEST when a field scorer references a missing field
3+
area: Ranking
4+
type: bug
5+
issues:
6+
- 127162

plugins/examples/rescore/src/yamlRestTest/resources/rest-api-spec/test/example-rescore/30_factor_field.yml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,24 @@ setup:
4848
- match: { hits.hits.1._score: 20 }
4949
- match: { hits.hits.2._score: 10 }
5050

51+
---
52+
"referencing a missing field returns bad request":
53+
- requires:
54+
cluster_features: [ "search.rescorer.missing.field.bad.request" ]
55+
reason: "Testing the behaviour change with this feature"
56+
- do:
57+
catch: bad_request
58+
search:
59+
index: test
60+
body:
61+
rescore:
62+
example:
63+
factor: 1
64+
factor_field: missing
65+
- match: { status: 400 }
66+
- match: { error.root_cause.0.type: "illegal_argument_exception" }
67+
- match: { error.root_cause.0.reason: "Missing value for field [missing]" }
68+
5169
---
5270
"sorted based on a numeric field and rescored based on a factor field using a window size":
5371
- do:

server/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
import org.apache.lucene.index.LeafReaderContext;
1313
import org.apache.lucene.search.Explanation;
14-
import org.elasticsearch.ElasticsearchException;
1514
import org.elasticsearch.common.io.stream.StreamInput;
1615
import org.elasticsearch.common.io.stream.StreamOutput;
1716
import org.elasticsearch.common.io.stream.Writeable;
@@ -73,7 +72,7 @@ public double score(int docId, float subQueryScore) throws IOException {
7372
if (missing != null) {
7473
value = missing;
7574
} else {
76-
throw new ElasticsearchException("Missing value for field [" + field + "]");
75+
throw new IllegalArgumentException("Missing value for field [" + field + "]");
7776
}
7877
}
7978
double val = value * boostFactor;

server/src/main/java/org/elasticsearch/search/SearchFeatures.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,10 @@ public Set<NodeFeature> getFeatures() {
2828
public static final NodeFeature COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS = new NodeFeature(
2929
"search.completion_field.duplicate.support"
3030
);
31+
public static final NodeFeature RESCORER_MISSING_FIELD_BAD_REQUEST = new NodeFeature("search.rescorer.missing.field.bad.request");
3132

3233
@Override
3334
public Set<NodeFeature> getTestFeatures() {
34-
return Set.of(RETRIEVER_RESCORER_ENABLED, COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS);
35+
return Set.of(RETRIEVER_RESCORER_ENABLED, COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS, RESCORER_MISSING_FIELD_BAD_REQUEST);
3536
}
3637
}

test/external-modules/multi-project/src/javaRestTest/java/org/elasticsearch/multiproject/action/DeleteProjectActionIT.java

Lines changed: 0 additions & 40 deletions
This file was deleted.
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.multiproject.action;
11+
12+
import org.elasticsearch.client.Request;
13+
import org.elasticsearch.client.Response;
14+
import org.elasticsearch.client.ResponseException;
15+
import org.elasticsearch.test.cluster.ElasticsearchCluster;
16+
import org.elasticsearch.test.cluster.local.distribution.DistributionType;
17+
import org.elasticsearch.test.rest.ESRestTestCase;
18+
import org.elasticsearch.test.rest.ObjectPath;
19+
import org.junit.ClassRule;
20+
21+
import java.io.IOException;
22+
import java.io.UncheckedIOException;
23+
import java.util.List;
24+
import java.util.Set;
25+
import java.util.concurrent.atomic.AtomicInteger;
26+
import java.util.stream.Collectors;
27+
import java.util.stream.IntStream;
28+
29+
import static org.hamcrest.CoreMatchers.containsString;
30+
import static org.hamcrest.CoreMatchers.equalTo;
31+
import static org.hamcrest.CoreMatchers.hasItem;
32+
import static org.hamcrest.CoreMatchers.not;
33+
34+
public class ProjectCrudActionIT extends ESRestTestCase {
35+
36+
@ClassRule
37+
public static ElasticsearchCluster CLUSTER = ElasticsearchCluster.local()
38+
.distribution(DistributionType.DEFAULT)
39+
.setting("test.multi_project.enabled", "true")
40+
.setting("xpack.security.enabled", "false")
41+
.build();
42+
43+
@Override
44+
protected String getTestRestCluster() {
45+
return CLUSTER.getHttpAddresses();
46+
}
47+
48+
public void testCreateAndDeleteProject() throws IOException {
49+
final var projectId = randomUniqueProjectId();
50+
var request = new Request("PUT", "/_project/" + projectId);
51+
52+
final int numberOfRequests = between(1, 8);
53+
final var successCount = new AtomicInteger();
54+
final var errorCount = new AtomicInteger();
55+
56+
runInParallel(numberOfRequests, ignore -> {
57+
try {
58+
var response = client().performRequest(request);
59+
assertAcknowledged(response);
60+
successCount.incrementAndGet();
61+
} catch (IOException e) {
62+
if (e instanceof ResponseException responseException) {
63+
assertThat(responseException.getMessage(), containsString("project [" + projectId + "] already exists"));
64+
errorCount.incrementAndGet();
65+
return;
66+
}
67+
fail(e, "unexpected exception");
68+
}
69+
});
70+
71+
assertThat(successCount.get(), equalTo(1));
72+
assertThat(errorCount.get(), equalTo(numberOfRequests - 1));
73+
assertThat(getProjectIdsFromClusterState(), hasItem(projectId.id()));
74+
75+
final Response response = client().performRequest(new Request("DELETE", "/_project/" + projectId));
76+
assertAcknowledged(response);
77+
assertThat(getProjectIdsFromClusterState(), not(hasItem(projectId.id())));
78+
}
79+
80+
private Set<String> getProjectIdsFromClusterState() throws IOException {
81+
final Response response = client().performRequest(new Request("GET", "/_cluster/state?multi_project=true"));
82+
final ObjectPath clusterState = assertOKAndCreateObjectPath(response);
83+
84+
final Set<String> projectIdsFromMetadata = extractProjectIds(clusterState, "metadata.projects");
85+
final Set<String> projectIdsFromRoutingTable = extractProjectIds(clusterState, "routing_table.projects");
86+
87+
assertThat(projectIdsFromMetadata, equalTo(projectIdsFromRoutingTable));
88+
return projectIdsFromMetadata;
89+
}
90+
91+
@SuppressWarnings("unchecked")
92+
private Set<String> extractProjectIds(ObjectPath clusterState, String path) throws IOException {
93+
final int numberProjects = ((List<Object>) clusterState.evaluate(path)).size();
94+
return IntStream.range(0, numberProjects).mapToObj(i -> {
95+
try {
96+
return (String) clusterState.evaluate(path + "." + i + ".id");
97+
} catch (IOException e) {
98+
throw new UncheckedIOException(e);
99+
}
100+
}).collect(Collectors.toUnmodifiableSet());
101+
}
102+
}

test/external-modules/multi-project/src/main/java/org/elasticsearch/multiproject/action/PutProjectAction.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
package org.elasticsearch.multiproject.action;
1111

12+
import org.elasticsearch.ResourceAlreadyExistsException;
1213
import org.elasticsearch.action.ActionListener;
1314
import org.elasticsearch.action.ActionRequestValidationException;
1415
import org.elasticsearch.action.ActionType;
@@ -37,6 +38,8 @@
3738
import org.elasticsearch.transport.TransportService;
3839

3940
import java.io.IOException;
41+
import java.util.HashSet;
42+
import java.util.Set;
4043
import java.util.regex.Pattern;
4144

4245
public class PutProjectAction extends ActionType<AcknowledgedResponse> {
@@ -105,11 +108,17 @@ static class PutProjectExecutor implements ClusterStateTaskExecutor<PutProjectTa
105108

106109
@Override
107110
public ClusterState execute(BatchExecutionContext<PutProjectTask> batchExecutionContext) throws Exception {
108-
var stateBuilder = ClusterState.builder(batchExecutionContext.initialState());
111+
final ClusterState initialState = batchExecutionContext.initialState();
112+
final Set<ProjectId> knownProjectIds = new HashSet<>(initialState.metadata().projects().keySet());
113+
var stateBuilder = ClusterState.builder(initialState);
109114
for (TaskContext<PutProjectTask> taskContext : batchExecutionContext.taskContexts()) {
110115
try {
111116
Request request = taskContext.getTask().request();
117+
if (knownProjectIds.contains(request.projectId)) {
118+
throw new ResourceAlreadyExistsException("project [{}] already exists", request.projectId);
119+
}
112120
stateBuilder.putProjectMetadata(ProjectMetadata.builder(request.projectId));
121+
knownProjectIds.add(request.projectId);
113122
taskContext.success(() -> taskContext.getTask().listener.onResponse(AcknowledgedResponse.TRUE));
114123
} catch (Exception e) {
115124
taskContext.onFailure(e);

0 commit comments

Comments
 (0)