Skip to content

Commit 3926f7a

Browse files
committed
feat(hunspell): Add ref_path support and cache invalidation API
- Add ref_path parameter for package-based dictionary loading - Load from config/packages/{packageId}/hunspell/{locale}/ - Add cache invalidation REST API (GET/POST /_hunspell/cache/_invalidate) - Add TransportAction with cluster:admin permission - Add comprehensive security validation (path traversal, separators, cache-key injection) - Add updateable flag for hot-reload via _reload_search_analyzers - Add comprehensive test coverage PR feedback addressed: - Stricter validate() to reject conflicting params - Path traversal checks now use config/packages/ as base - ref_path/locale validation rejects ., .., /, \, : characters Signed-off-by: shayush622 <ayush5267@gmail.com>
1 parent 9361d76 commit 3926f7a

File tree

18 files changed

+418
-121
lines changed

18 files changed

+418
-121
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,4 @@ testfixtures_shared/
6868

6969
# build files generated
7070
doc-tools/missing-doclet/bin/
71+
mise.toml

mise.toml

Lines changed: 0 additions & 2 deletions
This file was deleted.

server/src/main/java/org/opensearch/action/ActionModule.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@
138138
import org.opensearch.action.admin.indices.analyze.TransportAnalyzeAction;
139139
import org.opensearch.action.admin.indices.cache.clear.ClearIndicesCacheAction;
140140
import org.opensearch.action.admin.indices.cache.clear.TransportClearIndicesCacheAction;
141+
import org.opensearch.action.admin.indices.cache.hunspell.HunspellCacheInfoAction;
141142
import org.opensearch.action.admin.indices.cache.hunspell.HunspellCacheInvalidateAction;
143+
import org.opensearch.action.admin.indices.cache.hunspell.TransportHunspellCacheInfoAction;
142144
import org.opensearch.action.admin.indices.cache.hunspell.TransportHunspellCacheInvalidateAction;
143145
import org.opensearch.action.admin.indices.close.CloseIndexAction;
144146
import org.opensearch.action.admin.indices.close.TransportCloseIndexAction;
@@ -732,6 +734,7 @@ public <Request extends ActionRequest, Response extends ActionResponse> void reg
732734
actions.register(UpgradeStatusAction.INSTANCE, TransportUpgradeStatusAction.class);
733735
actions.register(UpgradeSettingsAction.INSTANCE, TransportUpgradeSettingsAction.class);
734736
actions.register(ClearIndicesCacheAction.INSTANCE, TransportClearIndicesCacheAction.class);
737+
actions.register(HunspellCacheInfoAction.INSTANCE, TransportHunspellCacheInfoAction.class);
735738
actions.register(HunspellCacheInvalidateAction.INSTANCE, TransportHunspellCacheInvalidateAction.class);
736739
actions.register(GetAliasesAction.INSTANCE, TransportGetAliasesAction.class);
737740
actions.register(GetSettingsAction.INSTANCE, TransportGetSettingsAction.class);
@@ -1085,9 +1088,7 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
10851088
registerHandler.accept(new RestGetIngestionStateAction());
10861089

10871090
// Hunspell cache management API
1088-
if (hunspellService != null) {
1089-
registerHandler.accept(new RestHunspellCacheInvalidateAction(hunspellService));
1090-
}
1091+
registerHandler.accept(new RestHunspellCacheInvalidateAction());
10911092
}
10921093

10931094
@Override
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.action.admin.indices.cache.hunspell;
10+
11+
import org.opensearch.action.ActionType;
12+
13+
/**
14+
* Action for retrieving Hunspell cache information.
15+
*
16+
* <p>This action requires "cluster:monitor/hunspell/cache" permission when security is enabled.
17+
*
18+
* @opensearch.internal
19+
*/
20+
public class HunspellCacheInfoAction extends ActionType<HunspellCacheInfoResponse> {
21+
22+
public static final HunspellCacheInfoAction INSTANCE = new HunspellCacheInfoAction();
23+
public static final String NAME = "cluster:monitor/hunspell/cache";
24+
25+
private HunspellCacheInfoAction() {
26+
super(NAME, HunspellCacheInfoResponse::new);
27+
}
28+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.action.admin.indices.cache.hunspell;
10+
11+
import org.opensearch.action.ActionRequest;
12+
import org.opensearch.action.ActionRequestValidationException;
13+
import org.opensearch.core.common.io.stream.StreamInput;
14+
import org.opensearch.core.common.io.stream.StreamOutput;
15+
16+
import java.io.IOException;
17+
18+
/**
19+
* Request for retrieving Hunspell cache information.
20+
*
21+
* @opensearch.internal
22+
*/
23+
public class HunspellCacheInfoRequest extends ActionRequest {
24+
25+
public HunspellCacheInfoRequest() {
26+
}
27+
28+
public HunspellCacheInfoRequest(StreamInput in) throws IOException {
29+
super(in);
30+
}
31+
32+
@Override
33+
public void writeTo(StreamOutput out) throws IOException {
34+
super.writeTo(out);
35+
}
36+
37+
@Override
38+
public ActionRequestValidationException validate() {
39+
return null;
40+
}
41+
}
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.action.admin.indices.cache.hunspell;
10+
11+
import org.opensearch.core.action.ActionResponse;
12+
import org.opensearch.core.common.io.stream.StreamInput;
13+
import org.opensearch.core.common.io.stream.StreamOutput;
14+
import org.opensearch.core.xcontent.ToXContentObject;
15+
import org.opensearch.core.xcontent.XContentBuilder;
16+
17+
import java.io.IOException;
18+
import java.util.HashSet;
19+
import java.util.Set;
20+
21+
/**
22+
* Response for Hunspell cache information.
23+
*
24+
* @opensearch.internal
25+
*/
26+
public class HunspellCacheInfoResponse extends ActionResponse implements ToXContentObject {
27+
28+
private final int totalCachedCount;
29+
private final int packageBasedCount;
30+
private final int traditionalLocaleCount;
31+
private final Set<String> packageBasedKeys;
32+
private final Set<String> traditionalLocaleKeys;
33+
34+
public HunspellCacheInfoResponse(
35+
int totalCachedCount,
36+
int packageBasedCount,
37+
int traditionalLocaleCount,
38+
Set<String> packageBasedKeys,
39+
Set<String> traditionalLocaleKeys
40+
) {
41+
this.totalCachedCount = totalCachedCount;
42+
this.packageBasedCount = packageBasedCount;
43+
this.traditionalLocaleCount = traditionalLocaleCount;
44+
this.packageBasedKeys = packageBasedKeys;
45+
this.traditionalLocaleKeys = traditionalLocaleKeys;
46+
}
47+
48+
public HunspellCacheInfoResponse(StreamInput in) throws IOException {
49+
super(in);
50+
this.totalCachedCount = in.readVInt();
51+
this.packageBasedCount = in.readVInt();
52+
this.traditionalLocaleCount = in.readVInt();
53+
this.packageBasedKeys = new HashSet<>(in.readStringList());
54+
this.traditionalLocaleKeys = new HashSet<>(in.readStringList());
55+
}
56+
57+
@Override
58+
public void writeTo(StreamOutput out) throws IOException {
59+
out.writeVInt(totalCachedCount);
60+
out.writeVInt(packageBasedCount);
61+
out.writeVInt(traditionalLocaleCount);
62+
out.writeStringCollection(packageBasedKeys);
63+
out.writeStringCollection(traditionalLocaleKeys);
64+
}
65+
66+
@Override
67+
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
68+
builder.startObject();
69+
builder.field("total_cached_count", totalCachedCount);
70+
builder.field("package_based_count", packageBasedCount);
71+
builder.field("traditional_locale_count", traditionalLocaleCount);
72+
builder.array("package_based_keys", packageBasedKeys.toArray(new String[0]));
73+
builder.array("traditional_locale_keys", traditionalLocaleKeys.toArray(new String[0]));
74+
builder.endObject();
75+
return builder;
76+
}
77+
78+
public int getTotalCachedCount() {
79+
return totalCachedCount;
80+
}
81+
82+
public int getPackageBasedCount() {
83+
return packageBasedCount;
84+
}
85+
86+
public int getTraditionalLocaleCount() {
87+
return traditionalLocaleCount;
88+
}
89+
90+
public Set<String> getPackageBasedKeys() {
91+
return packageBasedKeys;
92+
}
93+
94+
public Set<String> getTraditionalLocaleKeys() {
95+
return traditionalLocaleKeys;
96+
}
97+
}

server/src/main/java/org/opensearch/action/admin/indices/cache/hunspell/HunspellCacheInvalidateAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
* Action type for invalidating Hunspell dictionary cache.
1515
*
1616
* This action requires cluster admin permissions when the security plugin is enabled.
17-
* The action name "cluster:admin/hunspell/cache/clear" maps to IAM policies for authorization.
17+
* The action name "cluster:admin/hunspell/cache/clear" should be added to the cluster_permissions
18+
* section of a role definition for authorization.
1819
*
1920
* @opensearch.internal
2021
*/

server/src/main/java/org/opensearch/action/admin/indices/cache/hunspell/HunspellCacheInvalidateRequest.java

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import org.opensearch.action.ActionRequest;
1212
import org.opensearch.action.ActionRequestValidationException;
13+
import org.opensearch.core.common.Strings;
1314
import org.opensearch.core.common.io.stream.StreamInput;
1415
import org.opensearch.core.common.io.stream.StreamOutput;
1516

@@ -57,12 +58,52 @@ public void writeTo(StreamOutput out) throws IOException {
5758

5859
@Override
5960
public ActionRequestValidationException validate() {
60-
if (!invalidateAll && packageId == null && cacheKey == null) {
61-
ActionRequestValidationException e = new ActionRequestValidationException();
62-
e.addValidationError("Either 'package_id', 'cache_key', or 'invalidate_all' must be specified");
61+
ActionRequestValidationException e = null;
62+
63+
// Reject empty/blank strings with clear error messages
64+
if (packageId != null && !Strings.hasText(packageId)) {
65+
e = new ActionRequestValidationException();
66+
e.addValidationError("'package_id' cannot be empty or blank");
67+
}
68+
if (locale != null && !Strings.hasText(locale)) {
69+
if (e == null) e = new ActionRequestValidationException();
70+
e.addValidationError("'locale' cannot be empty or blank");
71+
}
72+
if (cacheKey != null && !Strings.hasText(cacheKey)) {
73+
if (e == null) e = new ActionRequestValidationException();
74+
e.addValidationError("'cache_key' cannot be empty or blank");
75+
}
76+
77+
// If any blank validation errors, return early
78+
if (e != null) {
6379
return e;
6480
}
65-
return null;
81+
82+
// Count how many modes are specified
83+
int modeCount = 0;
84+
if (invalidateAll) modeCount++;
85+
if (packageId != null) modeCount++;
86+
if (cacheKey != null) modeCount++;
87+
88+
if (modeCount == 0) {
89+
e = new ActionRequestValidationException();
90+
e.addValidationError("Either 'package_id', 'cache_key', or 'invalidate_all' must be specified");
91+
} else if (modeCount > 1) {
92+
e = new ActionRequestValidationException();
93+
if (invalidateAll && (packageId != null || cacheKey != null)) {
94+
e.addValidationError("'invalidate_all' cannot be combined with 'package_id' or 'cache_key'");
95+
} else {
96+
e.addValidationError("Only one of 'package_id' or 'cache_key' can be specified, not both");
97+
}
98+
}
99+
100+
// locale is only valid with package_id
101+
if (locale != null && packageId == null) {
102+
if (e == null) e = new ActionRequestValidationException();
103+
e.addValidationError("'locale' can only be specified together with 'package_id'");
104+
}
105+
106+
return e;
66107
}
67108

68109
public String getPackageId() {

server/src/main/java/org/opensearch/action/admin/indices/cache/hunspell/HunspellCacheInvalidateResponse.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
8080
builder.startObject();
8181
builder.field("acknowledged", acknowledged);
8282
builder.field("invalidated_count", invalidatedCount);
83-
if (packageId != null) {
84-
builder.field("package_id", packageId);
85-
}
86-
if (locale != null) {
87-
builder.field("locale", locale);
88-
}
89-
if (cacheKey != null) {
90-
builder.field("cache_key", cacheKey);
91-
}
83+
builder.field("package_id", packageId);
84+
builder.field("locale", locale);
85+
builder.field("cache_key", cacheKey);
9286
builder.endObject();
9387
return builder;
9488
}
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.action.admin.indices.cache.hunspell;
10+
11+
import org.opensearch.action.support.ActionFilters;
12+
import org.opensearch.action.support.HandledTransportAction;
13+
import org.opensearch.common.inject.Inject;
14+
import org.opensearch.core.action.ActionListener;
15+
import org.opensearch.indices.analysis.HunspellService;
16+
import org.opensearch.tasks.Task;
17+
import org.opensearch.transport.TransportService;
18+
19+
import java.util.HashSet;
20+
import java.util.Set;
21+
22+
/**
23+
* Transport action for retrieving Hunspell cache information.
24+
*
25+
* <p>Requires "cluster:monitor/hunspell/cache" permission when security is enabled.
26+
*
27+
* @opensearch.internal
28+
*/
29+
public class TransportHunspellCacheInfoAction extends HandledTransportAction<HunspellCacheInfoRequest, HunspellCacheInfoResponse> {
30+
31+
private final HunspellService hunspellService;
32+
33+
@Inject
34+
public TransportHunspellCacheInfoAction(
35+
TransportService transportService,
36+
ActionFilters actionFilters,
37+
HunspellService hunspellService
38+
) {
39+
super(HunspellCacheInfoAction.NAME, transportService, actionFilters, HunspellCacheInfoRequest::new);
40+
this.hunspellService = hunspellService;
41+
}
42+
43+
@Override
44+
protected void doExecute(Task task, HunspellCacheInfoRequest request, ActionListener<HunspellCacheInfoResponse> listener) {
45+
try {
46+
Set<String> cachedKeys = hunspellService.getCachedDictionaryKeys();
47+
48+
Set<String> packageKeys = new HashSet<>();
49+
Set<String> localeKeys = new HashSet<>();
50+
51+
for (String key : cachedKeys) {
52+
if (HunspellService.isPackageCacheKey(key)) {
53+
packageKeys.add(key);
54+
} else {
55+
localeKeys.add(key);
56+
}
57+
}
58+
59+
HunspellCacheInfoResponse response = new HunspellCacheInfoResponse(
60+
cachedKeys.size(),
61+
packageKeys.size(),
62+
localeKeys.size(),
63+
packageKeys,
64+
localeKeys
65+
);
66+
67+
listener.onResponse(response);
68+
} catch (Exception e) {
69+
listener.onFailure(e);
70+
}
71+
}
72+
}

0 commit comments

Comments
 (0)