Skip to content

Commit 93df1ee

Browse files
DaveCTurnerDaveCTurner
authored
Make PutStoredScriptRequest immutable (elastic#117556) (elastic#117566)
No need for this request to be mutable, we always know all the values at creation time. Also adjusts the `toString()` impl to use the `source` field, since this is the only spot that we use the `content` so with this change we can follow up with a 9.x-only change to remove it.
1 parent 993add6 commit 93df1ee

File tree

6 files changed

+60
-106
lines changed

6 files changed

+60
-106
lines changed

‎modules/lang-mustache/src/internalClusterTest/java/org/elasticsearch/script/mustache/SearchTemplateIT.java‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,10 @@
1313
import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptAction;
1414
import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptRequest;
1515
import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptResponse;
16-
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
1716
import org.elasticsearch.action.admin.cluster.storedscripts.TransportDeleteStoredScriptAction;
1817
import org.elasticsearch.action.admin.cluster.storedscripts.TransportPutStoredScriptAction;
1918
import org.elasticsearch.action.bulk.BulkRequestBuilder;
2019
import org.elasticsearch.action.search.SearchRequest;
21-
import org.elasticsearch.common.bytes.BytesArray;
2220
import org.elasticsearch.common.settings.Settings;
2321
import org.elasticsearch.plugins.Plugin;
2422
import org.elasticsearch.script.ScriptType;
@@ -39,6 +37,7 @@
3937
import java.util.Map;
4038
import java.util.concurrent.ExecutionException;
4139

40+
import static org.elasticsearch.action.admin.cluster.storedscripts.StoredScriptIntegTestUtils.newPutStoredScriptTestRequest;
4241
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
4342
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;
4443
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
@@ -467,12 +466,6 @@ public static void assertHitCount(SearchTemplateRequestBuilder requestBuilder, l
467466
}
468467

469468
private void putJsonStoredScript(String id, String jsonContent) {
470-
assertAcked(
471-
safeExecute(
472-
TransportPutStoredScriptAction.TYPE,
473-
new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT).id(id)
474-
.content(new BytesArray(jsonContent), XContentType.JSON)
475-
)
476-
);
469+
assertAcked(safeExecute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest(id, jsonContent)));
477470
}
478471
}

‎server/src/internalClusterTest/java/org/elasticsearch/script/StoredScriptsIT.java‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,21 @@
1111
import org.elasticsearch.action.admin.cluster.storedscripts.DeleteStoredScriptRequest;
1212
import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptAction;
1313
import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptRequest;
14-
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
1514
import org.elasticsearch.action.admin.cluster.storedscripts.TransportDeleteStoredScriptAction;
1615
import org.elasticsearch.action.admin.cluster.storedscripts.TransportPutStoredScriptAction;
1716
import org.elasticsearch.action.support.master.AcknowledgedResponse;
18-
import org.elasticsearch.common.bytes.BytesArray;
1917
import org.elasticsearch.common.settings.Settings;
2018
import org.elasticsearch.core.Strings;
2119
import org.elasticsearch.plugins.Plugin;
2220
import org.elasticsearch.test.ESIntegTestCase;
23-
import org.elasticsearch.xcontent.XContentType;
2421

2522
import java.util.Arrays;
2623
import java.util.Collection;
2724
import java.util.Collections;
2825
import java.util.Map;
2926
import java.util.function.Function;
3027

28+
import static org.elasticsearch.action.admin.cluster.storedscripts.StoredScriptIntegTestUtils.newPutStoredScriptTestRequest;
3129
import static org.elasticsearch.action.admin.cluster.storedscripts.StoredScriptIntegTestUtils.putJsonStoredScript;
3230
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
3331

@@ -73,14 +71,9 @@ public void testBasics() {
7371
safeAwaitAndUnwrapFailure(
7472
IllegalArgumentException.class,
7573
AcknowledgedResponse.class,
76-
l -> client().execute(
77-
TransportPutStoredScriptAction.TYPE,
78-
new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT).id("id#")
79-
.content(new BytesArray(Strings.format("""
80-
{"script": {"lang": "%s", "source": "1"} }
81-
""", LANG)), XContentType.JSON),
82-
l
83-
)
74+
l -> client().execute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest("id#", Strings.format("""
75+
{"script": {"lang": "%s", "source": "1"} }
76+
""", LANG)), l)
8477
).getMessage()
8578
);
8679
}
@@ -91,14 +84,9 @@ public void testMaxScriptSize() {
9184
safeAwaitAndUnwrapFailure(
9285
IllegalArgumentException.class,
9386
AcknowledgedResponse.class,
94-
l -> client().execute(
95-
TransportPutStoredScriptAction.TYPE,
96-
new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT).id("foobar")
97-
.content(new BytesArray(Strings.format("""
98-
{"script": { "lang": "%s", "source":"0123456789abcdef"} }\
99-
""", LANG)), XContentType.JSON),
100-
l
101-
)
87+
l -> client().execute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest("foobar", Strings.format("""
88+
{"script": { "lang": "%s", "source":"0123456789abcdef"} }\
89+
""", LANG)), l)
10290
).getMessage()
10391
);
10492
}

‎server/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequest.java‎

Lines changed: 22 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@
1111

1212
import org.elasticsearch.action.ActionRequestValidationException;
1313
import org.elasticsearch.action.support.master.AcknowledgedRequest;
14+
import org.elasticsearch.common.Strings;
1415
import org.elasticsearch.common.bytes.BytesReference;
1516
import org.elasticsearch.common.io.stream.StreamInput;
1617
import org.elasticsearch.common.io.stream.StreamOutput;
1718
import org.elasticsearch.common.xcontent.XContentHelper;
19+
import org.elasticsearch.core.Nullable;
1820
import org.elasticsearch.core.TimeValue;
1921
import org.elasticsearch.script.StoredScriptSource;
2022
import org.elasticsearch.xcontent.ToXContentFragment;
@@ -28,11 +30,15 @@
2830

2931
public class PutStoredScriptRequest extends AcknowledgedRequest<PutStoredScriptRequest> implements ToXContentFragment {
3032

31-
private String id;
32-
private String context;
33-
private BytesReference content;
34-
private XContentType xContentType;
35-
private StoredScriptSource source;
33+
@Nullable
34+
private final String id;
35+
36+
@Nullable
37+
private final String context;
38+
39+
private final BytesReference content;
40+
private final XContentType xContentType;
41+
private final StoredScriptSource source;
3642

3743
public PutStoredScriptRequest(StreamInput in) throws IOException {
3844
super(in);
@@ -43,25 +49,21 @@ public PutStoredScriptRequest(StreamInput in) throws IOException {
4349
source = new StoredScriptSource(in);
4450
}
4551

46-
public PutStoredScriptRequest(TimeValue masterNodeTimeout, TimeValue ackTimeout) {
47-
super(masterNodeTimeout, ackTimeout);
48-
}
49-
5052
public PutStoredScriptRequest(
5153
TimeValue masterNodeTimeout,
5254
TimeValue ackTimeout,
53-
String id,
54-
String context,
55+
@Nullable String id,
56+
@Nullable String context,
5557
BytesReference content,
5658
XContentType xContentType,
5759
StoredScriptSource source
5860
) {
5961
super(masterNodeTimeout, ackTimeout);
6062
this.id = id;
6163
this.context = context;
62-
this.content = content;
64+
this.content = Objects.requireNonNull(content);
6365
this.xContentType = Objects.requireNonNull(xContentType);
64-
this.source = source;
66+
this.source = Objects.requireNonNull(source);
6567
}
6668

6769
@Override
@@ -74,31 +76,17 @@ public ActionRequestValidationException validate() {
7476
validationException = addValidationError("id cannot contain '#' for stored script", validationException);
7577
}
7678

77-
if (content == null) {
78-
validationException = addValidationError("must specify code for stored script", validationException);
79-
}
80-
8179
return validationException;
8280
}
8381

8482
public String id() {
8583
return id;
8684
}
8785

88-
public PutStoredScriptRequest id(String id) {
89-
this.id = id;
90-
return this;
91-
}
92-
9386
public String context() {
9487
return context;
9588
}
9689

97-
public PutStoredScriptRequest context(String context) {
98-
this.context = context;
99-
return this;
100-
}
101-
10290
public BytesReference content() {
10391
return content;
10492
}
@@ -111,16 +99,6 @@ public StoredScriptSource source() {
11199
return source;
112100
}
113101

114-
/**
115-
* Set the script source and the content type of the bytes.
116-
*/
117-
public PutStoredScriptRequest content(BytesReference content, XContentType xContentType) {
118-
this.content = content;
119-
this.xContentType = Objects.requireNonNull(xContentType);
120-
this.source = StoredScriptSource.parse(content, xContentType);
121-
return this;
122-
}
123-
124102
@Override
125103
public void writeTo(StreamOutput out) throws IOException {
126104
super.writeTo(out);
@@ -133,28 +111,16 @@ public void writeTo(StreamOutput out) throws IOException {
133111

134112
@Override
135113
public String toString() {
136-
String source = "_na_";
137-
138-
try {
139-
source = XContentHelper.convertToJson(content, false, xContentType);
140-
} catch (Exception e) {
141-
// ignore
142-
}
143-
144-
return "put stored script {id ["
145-
+ id
146-
+ "]"
147-
+ (context != null ? ", context [" + context + "]" : "")
148-
+ ", content ["
149-
+ source
150-
+ "]}";
114+
return Strings.format(
115+
"put stored script {id [%s]%s, content [%s]}",
116+
id,
117+
context != null ? ", context [" + context + "]" : "",
118+
source
119+
);
151120
}
152121

153122
@Override
154123
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
155-
builder.field("script");
156-
source.toXContent(builder, params);
157-
158-
return builder;
124+
return builder.field("script", source, params);
159125
}
160126
}

‎server/src/test/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequestTests.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,15 @@ public void testToXContent() throws IOException {
5757

5858
BytesReference expectedRequestBody = BytesReference.bytes(builder);
5959

60-
PutStoredScriptRequest request = new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT);
61-
request.id("test1");
62-
request.content(expectedRequestBody, xContentType);
60+
PutStoredScriptRequest request = new PutStoredScriptRequest(
61+
TEST_REQUEST_TIMEOUT,
62+
TEST_REQUEST_TIMEOUT,
63+
"test1",
64+
null,
65+
expectedRequestBody,
66+
xContentType,
67+
StoredScriptSource.parse(expectedRequestBody, xContentType)
68+
);
6369

6470
XContentBuilder requestBuilder = XContentBuilder.builder(xContentType.xContent());
6571
requestBuilder.startObject();

‎test/framework/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/StoredScriptIntegTestUtils.java‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import org.elasticsearch.common.bytes.BytesArray;
1313
import org.elasticsearch.common.bytes.BytesReference;
14+
import org.elasticsearch.script.StoredScriptSource;
1415
import org.elasticsearch.test.ESIntegTestCase;
1516
import org.elasticsearch.xcontent.XContentType;
1617

@@ -25,11 +26,22 @@ public static void putJsonStoredScript(String id, String jsonContent) {
2526
}
2627

2728
public static void putJsonStoredScript(String id, BytesReference jsonContent) {
28-
assertAcked(
29-
ESIntegTestCase.safeExecute(
30-
TransportPutStoredScriptAction.TYPE,
31-
new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT).id(id).content(jsonContent, XContentType.JSON)
32-
)
29+
assertAcked(ESIntegTestCase.safeExecute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest(id, jsonContent)));
30+
}
31+
32+
public static PutStoredScriptRequest newPutStoredScriptTestRequest(String id, String jsonContent) {
33+
return newPutStoredScriptTestRequest(id, new BytesArray(jsonContent));
34+
}
35+
36+
public static PutStoredScriptRequest newPutStoredScriptTestRequest(String id, BytesReference jsonContent) {
37+
return new PutStoredScriptRequest(
38+
TEST_REQUEST_TIMEOUT,
39+
TEST_REQUEST_TIMEOUT,
40+
id,
41+
null,
42+
jsonContent,
43+
XContentType.JSON,
44+
StoredScriptSource.parse(jsonContent, XContentType.JSON)
3345
);
3446
}
3547
}

‎x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DlsFlsRequestCacheTests.java‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,11 @@
88
package org.elasticsearch.integration;
99

1010
import org.elasticsearch.ElasticsearchSecurityException;
11-
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
1211
import org.elasticsearch.action.admin.cluster.storedscripts.TransportPutStoredScriptAction;
1312
import org.elasticsearch.action.admin.indices.alias.Alias;
1413
import org.elasticsearch.action.search.SearchRequestBuilder;
1514
import org.elasticsearch.action.support.broadcast.BroadcastResponse;
1615
import org.elasticsearch.client.internal.Client;
17-
import org.elasticsearch.common.bytes.BytesArray;
1816
import org.elasticsearch.common.settings.SecureString;
1917
import org.elasticsearch.common.settings.Settings;
2018
import org.elasticsearch.core.Strings;
@@ -24,7 +22,6 @@
2422
import org.elasticsearch.search.SearchHit;
2523
import org.elasticsearch.test.SecuritySingleNodeTestCase;
2624
import org.elasticsearch.test.hamcrest.ElasticsearchAssertions;
27-
import org.elasticsearch.xcontent.XContentType;
2825
import org.elasticsearch.xpack.core.XPackSettings;
2926
import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyAction;
3027
import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyRequest;
@@ -43,6 +40,7 @@
4340
import java.util.concurrent.ExecutionException;
4441
import java.util.stream.Collectors;
4542

43+
import static org.elasticsearch.action.admin.cluster.storedscripts.StoredScriptIntegTestUtils.newPutStoredScriptTestRequest;
4644
import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
4745
import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.NONE;
4846
import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.WAIT_UNTIL;
@@ -350,17 +348,8 @@ public void testRequestCacheWithTemplateRoleQuery() {
350348
private void prepareIndices() {
351349
final Client client = client();
352350

353-
assertAcked(
354-
safeExecute(
355-
TransportPutStoredScriptAction.TYPE,
356-
new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT).id("my-script")
357-
.content(
358-
new BytesArray("""
359-
{"script":{"source":"{\\"match\\":{\\"username\\":\\"{{_user.username}}\\"}}","lang":"mustache"}}"""),
360-
XContentType.JSON
361-
)
362-
)
363-
);
351+
assertAcked(safeExecute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest("my-script", """
352+
{"script":{"source":"{\\"match\\":{\\"username\\":\\"{{_user.username}}\\"}}","lang":"mustache"}}""")));
364353

365354
assertAcked(indicesAdmin().prepareCreate(DLS_INDEX).addAlias(new Alias("dls-alias")).get());
366355
client.prepareIndex(DLS_INDEX).setId("101").setSource("number", 101, "letter", "A").get();

0 commit comments

Comments
 (0)