-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat(analysis): Add package-based Hunspell dictionary support with ref_path parameter and cache invalidation API #20741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
865aab5
1be4dde
c114694
9361d76
3926f7a
c1dd00b
20ba2c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,3 +68,4 @@ testfixtures_shared/ | |
|
|
||
| # build files generated | ||
| doc-tools/missing-doclet/bin/ | ||
| mise.toml | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| /* | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * The OpenSearch Contributors require contributions made to | ||
| * this file be licensed under the Apache-2.0 license or a | ||
| * compatible open source license. | ||
| */ | ||
|
|
||
| package org.opensearch.action.admin.indices.cache.hunspell; | ||
|
|
||
| import org.opensearch.common.settings.Settings; | ||
| import org.opensearch.indices.analysis.HunspellService; | ||
| import org.opensearch.test.OpenSearchIntegTestCase; | ||
| import org.opensearch.test.OpenSearchIntegTestCase.ClusterScope; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
|
|
||
| import static org.hamcrest.Matchers.equalTo; | ||
| import static org.hamcrest.Matchers.greaterThanOrEqualTo; | ||
|
|
||
| /** | ||
| * Integration tests for hunspell cache info and invalidation APIs. | ||
| * | ||
| * <p>Tests the full REST→Transport→HunspellService flow on a real cluster: | ||
| * <ul> | ||
| * <li>GET /_hunspell/cache (HunspellCacheInfoAction)</li> | ||
| * <li>POST /_hunspell/cache/_invalidate (HunspellCacheInvalidateAction)</li> | ||
| * </ul> | ||
| */ | ||
| @ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST) | ||
| public class HunspellCacheIT extends OpenSearchIntegTestCase { | ||
|
|
||
| @Override | ||
| protected Settings nodeSettings(int nodeOrdinal) { | ||
| // Enable lazy loading so dictionaries are only loaded on-demand | ||
| return Settings.builder().put(super.nodeSettings(nodeOrdinal)).put(HunspellService.HUNSPELL_LAZY_LOAD.getKey(), true).build(); | ||
| } | ||
|
|
||
| /** | ||
| * Sets up hunspell dictionary files in the node's config directory. | ||
| * Creates both traditional and package-based dictionaries. | ||
| */ | ||
| private void setupDictionaries(Path configDir) throws IOException { | ||
| // Traditional dictionary: config/hunspell/en_US/ | ||
| Path traditionalDir = configDir.resolve("hunspell").resolve("en_US"); | ||
| Files.createDirectories(traditionalDir); | ||
| Files.write(traditionalDir.resolve("en_US.aff"), "SET UTF-8\n".getBytes(StandardCharsets.UTF_8)); | ||
| Files.write(traditionalDir.resolve("en_US.dic"), "1\nhello\n".getBytes(StandardCharsets.UTF_8)); | ||
|
|
||
| // Package-based dictionary: config/packages/test-pkg/hunspell/en_US/ | ||
| Path packageDir = configDir.resolve("packages").resolve("test-pkg").resolve("hunspell").resolve("en_US"); | ||
| Files.createDirectories(packageDir); | ||
| Files.write(packageDir.resolve("en_US.aff"), "SET UTF-8\n".getBytes(StandardCharsets.UTF_8)); | ||
| Files.write(packageDir.resolve("en_US.dic"), "1\nworld\n".getBytes(StandardCharsets.UTF_8)); | ||
| } | ||
|
|
||
| // ==================== Cache Info Tests ==================== | ||
|
|
||
| public void testCacheInfoReturnsEmptyWhenNoDictionariesLoaded() { | ||
| HunspellCacheInfoResponse response = client().execute(HunspellCacheInfoAction.INSTANCE, new HunspellCacheInfoRequest()).actionGet(); | ||
|
|
||
| assertThat(response.getTotalCachedCount(), equalTo(0)); | ||
| assertThat(response.getPackageBasedCount(), equalTo(0)); | ||
| assertThat(response.getTraditionalLocaleCount(), equalTo(0)); | ||
| assertTrue(response.getPackageBasedKeys().isEmpty()); | ||
| assertTrue(response.getTraditionalLocaleKeys().isEmpty()); | ||
| } | ||
|
|
||
| // ==================== Cache Invalidation Tests ==================== | ||
|
|
||
| public void testInvalidateAllOnEmptyCache() { | ||
| HunspellCacheInvalidateRequest request = new HunspellCacheInvalidateRequest(); | ||
| request.setInvalidateAll(true); | ||
|
|
||
| HunspellCacheInvalidateResponse response = client().execute(HunspellCacheInvalidateAction.INSTANCE, request).actionGet(); | ||
|
|
||
| assertTrue(response.isAcknowledged()); | ||
| assertThat(response.getInvalidatedCount(), equalTo(0)); | ||
| } | ||
|
|
||
| public void testInvalidateByPackageIdOnEmptyCache() { | ||
| HunspellCacheInvalidateRequest request = new HunspellCacheInvalidateRequest(); | ||
| request.setPackageId("nonexistent-pkg"); | ||
|
|
||
| HunspellCacheInvalidateResponse response = client().execute(HunspellCacheInvalidateAction.INSTANCE, request).actionGet(); | ||
|
|
||
| assertTrue(response.isAcknowledged()); | ||
| assertThat(response.getInvalidatedCount(), equalTo(0)); | ||
| assertThat(response.getPackageId(), equalTo("nonexistent-pkg")); | ||
| } | ||
|
|
||
| public void testInvalidateByCacheKeyOnEmptyCache() { | ||
| HunspellCacheInvalidateRequest request = new HunspellCacheInvalidateRequest(); | ||
| request.setCacheKey("nonexistent-key"); | ||
|
|
||
| HunspellCacheInvalidateResponse response = client().execute(HunspellCacheInvalidateAction.INSTANCE, request).actionGet(); | ||
|
|
||
| assertTrue(response.isAcknowledged()); | ||
| assertThat(response.getInvalidatedCount(), equalTo(0)); | ||
| } | ||
|
|
||
| // ==================== Request Validation Tests ==================== | ||
|
|
||
| public void testInvalidateRequestValidationFailsWithNoParameters() { | ||
| HunspellCacheInvalidateRequest request = new HunspellCacheInvalidateRequest(); | ||
| // No parameters set — should fail validation | ||
|
|
||
| assertNotNull(request.validate()); | ||
| } | ||
|
|
||
| public void testInvalidateRequestValidationFailsWithConflictingParameters() { | ||
| HunspellCacheInvalidateRequest request = new HunspellCacheInvalidateRequest(); | ||
| request.setInvalidateAll(true); | ||
| request.setPackageId("some-pkg"); | ||
|
|
||
| assertNotNull(request.validate()); | ||
| } | ||
|
|
||
| public void testInvalidateRequestValidationFailsWithEmptyPackageId() { | ||
| HunspellCacheInvalidateRequest request = new HunspellCacheInvalidateRequest(); | ||
| request.setPackageId(" "); | ||
|
|
||
| assertNotNull(request.validate()); | ||
| } | ||
|
|
||
| public void testInvalidateRequestValidationPassesWithPackageIdOnly() { | ||
| HunspellCacheInvalidateRequest request = new HunspellCacheInvalidateRequest(); | ||
| request.setPackageId("valid-pkg"); | ||
|
|
||
| assertNull(request.validate()); | ||
| } | ||
|
|
||
| public void testInvalidateRequestValidationPassesWithPackageIdAndLocale() { | ||
| HunspellCacheInvalidateRequest request = new HunspellCacheInvalidateRequest(); | ||
| request.setPackageId("valid-pkg"); | ||
| request.setLocale("en_US"); | ||
|
|
||
| assertNull(request.validate()); | ||
| } | ||
|
|
||
| public void testInvalidateRequestValidationFailsWithLocaleWithoutPackageId() { | ||
| HunspellCacheInvalidateRequest request = new HunspellCacheInvalidateRequest(); | ||
| request.setLocale("en_US"); | ||
|
|
||
| assertNotNull(request.validate()); | ||
| } | ||
|
|
||
| // ==================== Response Schema Tests ==================== | ||
|
|
||
| public void testInvalidateResponseAlwaysIncludesAllFields() { | ||
| HunspellCacheInvalidateRequest request = new HunspellCacheInvalidateRequest(); | ||
| request.setInvalidateAll(true); | ||
|
|
||
| HunspellCacheInvalidateResponse response = client().execute(HunspellCacheInvalidateAction.INSTANCE, request).actionGet(); | ||
|
|
||
| // Response should always include these fields for consistent schema | ||
| assertTrue(response.isAcknowledged()); | ||
| assertThat(response.getInvalidatedCount(), greaterThanOrEqualTo(0)); | ||
| // Null fields should still be accessible (consistent schema) | ||
| assertNull(response.getPackageId()); | ||
| assertNull(response.getLocale()); | ||
| assertNull(response.getCacheKey()); | ||
| } | ||
|
|
||
| public void testCacheInfoResponseSchema() { | ||
| HunspellCacheInfoResponse response = client().execute(HunspellCacheInfoAction.INSTANCE, new HunspellCacheInfoRequest()).actionGet(); | ||
|
|
||
| // Verify response schema has all expected fields | ||
| assertThat(response.getTotalCachedCount(), greaterThanOrEqualTo(0)); | ||
| assertThat(response.getPackageBasedCount(), greaterThanOrEqualTo(0)); | ||
| assertThat(response.getTraditionalLocaleCount(), greaterThanOrEqualTo(0)); | ||
| assertNotNull(response.getPackageBasedKeys()); | ||
| assertNotNull(response.getTraditionalLocaleKeys()); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -138,6 +138,10 @@ | |||||||||
| import org.opensearch.action.admin.indices.analyze.TransportAnalyzeAction; | ||||||||||
| import org.opensearch.action.admin.indices.cache.clear.ClearIndicesCacheAction; | ||||||||||
| import org.opensearch.action.admin.indices.cache.clear.TransportClearIndicesCacheAction; | ||||||||||
| import org.opensearch.action.admin.indices.cache.hunspell.HunspellCacheInfoAction; | ||||||||||
| import org.opensearch.action.admin.indices.cache.hunspell.HunspellCacheInvalidateAction; | ||||||||||
| import org.opensearch.action.admin.indices.cache.hunspell.TransportHunspellCacheInfoAction; | ||||||||||
| import org.opensearch.action.admin.indices.cache.hunspell.TransportHunspellCacheInvalidateAction; | ||||||||||
| import org.opensearch.action.admin.indices.close.CloseIndexAction; | ||||||||||
| import org.opensearch.action.admin.indices.close.TransportCloseIndexAction; | ||||||||||
| import org.opensearch.action.admin.indices.create.AutoCreateAction; | ||||||||||
|
|
@@ -333,6 +337,7 @@ | |||||||||
| import org.opensearch.identity.IdentityService; | ||||||||||
| import org.opensearch.index.seqno.RetentionLeaseActions; | ||||||||||
| import org.opensearch.indices.SystemIndices; | ||||||||||
| import org.opensearch.indices.analysis.HunspellService; | ||||||||||
| import org.opensearch.persistent.CompletionPersistentTaskAction; | ||||||||||
| import org.opensearch.persistent.RemovePersistentTaskAction; | ||||||||||
| import org.opensearch.persistent.StartPersistentTaskAction; | ||||||||||
|
|
@@ -419,6 +424,7 @@ | |||||||||
| import org.opensearch.rest.action.admin.indices.RestGetIngestionStateAction; | ||||||||||
| import org.opensearch.rest.action.admin.indices.RestGetMappingAction; | ||||||||||
| import org.opensearch.rest.action.admin.indices.RestGetSettingsAction; | ||||||||||
| import org.opensearch.rest.action.admin.indices.RestHunspellCacheInvalidateAction; | ||||||||||
| import org.opensearch.rest.action.admin.indices.RestIndexDeleteAliasesAction; | ||||||||||
| import org.opensearch.rest.action.admin.indices.RestIndexPutAliasAction; | ||||||||||
| import org.opensearch.rest.action.admin.indices.RestIndicesAliasesAction; | ||||||||||
|
|
@@ -557,6 +563,7 @@ public class ActionModule extends AbstractModule { | |||||||||
| private final ThreadPool threadPool; | ||||||||||
| private final ExtensionsManager extensionsManager; | ||||||||||
| private final ResponseLimitSettings responseLimitSettings; | ||||||||||
| private final HunspellService hunspellService; | ||||||||||
|
|
||||||||||
| public ActionModule( | ||||||||||
| Settings settings, | ||||||||||
|
|
@@ -571,7 +578,8 @@ public ActionModule( | |||||||||
| UsageService usageService, | ||||||||||
| SystemIndices systemIndices, | ||||||||||
| IdentityService identityService, | ||||||||||
| ExtensionsManager extensionsManager | ||||||||||
| ExtensionsManager extensionsManager, | ||||||||||
| HunspellService hunspellService | ||||||||||
| ) { | ||||||||||
| this.settings = settings; | ||||||||||
| this.indexNameExpressionResolver = indexNameExpressionResolver; | ||||||||||
|
|
@@ -581,6 +589,7 @@ public ActionModule( | |||||||||
| this.actionPlugins = actionPlugins; | ||||||||||
| this.threadPool = threadPool; | ||||||||||
| this.extensionsManager = extensionsManager; | ||||||||||
| this.hunspellService = hunspellService; | ||||||||||
| actions = setupActions(actionPlugins); | ||||||||||
shayush622 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
| actionFilters = setupActionFilters(actionPlugins); | ||||||||||
| dynamicActionRegistry = new DynamicActionRegistry(); | ||||||||||
|
|
@@ -725,6 +734,8 @@ public <Request extends ActionRequest, Response extends ActionResponse> void reg | |||||||||
| actions.register(UpgradeStatusAction.INSTANCE, TransportUpgradeStatusAction.class); | ||||||||||
| actions.register(UpgradeSettingsAction.INSTANCE, TransportUpgradeSettingsAction.class); | ||||||||||
| actions.register(ClearIndicesCacheAction.INSTANCE, TransportClearIndicesCacheAction.class); | ||||||||||
| actions.register(HunspellCacheInfoAction.INSTANCE, TransportHunspellCacheInfoAction.class); | ||||||||||
| actions.register(HunspellCacheInvalidateAction.INSTANCE, TransportHunspellCacheInvalidateAction.class); | ||||||||||
| actions.register(GetAliasesAction.INSTANCE, TransportGetAliasesAction.class); | ||||||||||
| actions.register(GetSettingsAction.INSTANCE, TransportGetSettingsAction.class); | ||||||||||
|
|
||||||||||
|
|
@@ -1075,6 +1086,9 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) { | |||||||||
| registerHandler.accept(new RestPauseIngestionAction()); | ||||||||||
| registerHandler.accept(new RestResumeIngestionAction()); | ||||||||||
| registerHandler.accept(new RestGetIngestionStateAction()); | ||||||||||
|
|
||||||||||
| // Hunspell cache management API | ||||||||||
| registerHandler.accept(new RestHunspellCacheInvalidateAction()); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| @Override | ||||||||||
|
|
@@ -1110,6 +1124,11 @@ protected void configure() { | |||||||||
| bind(DynamicActionRegistry.class).toInstance(dynamicActionRegistry); | ||||||||||
|
|
||||||||||
| bind(ResponseLimitSettings.class).toInstance(responseLimitSettings); | ||||||||||
|
|
||||||||||
| // Bind HunspellService for TransportHunspellCacheInvalidateAction injection | ||||||||||
| if (hunspellService != null) { | ||||||||||
| bind(HunspellService.class).toInstance(hunspellService); | ||||||||||
| } | ||||||||||
|
Comment on lines
+1129
to
+1131
|
||||||||||
| if (hunspellService != null) { | |
| bind(HunspellService.class).toInstance(hunspellService); | |
| } | |
| bind(HunspellService.class).toInstance(requireNonNull(hunspellService, "hunspellService must not be null")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ can hunspellService ever be null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ do this in Node.java directly.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| /* | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * The OpenSearch Contributors require contributions made to | ||
| * this file be licensed under the Apache-2.0 license or a | ||
| * compatible open source license. | ||
| */ | ||
|
|
||
| package org.opensearch.action.admin.indices.cache.hunspell; | ||
|
|
||
| import org.opensearch.action.ActionType; | ||
|
|
||
| /** | ||
| * Action for retrieving Hunspell cache information. | ||
| * | ||
| * <p>This action requires "cluster:monitor/hunspell/cache" permission when security is enabled. | ||
| * | ||
| * @opensearch.internal | ||
| */ | ||
| public class HunspellCacheInfoAction extends ActionType<HunspellCacheInfoResponse> { | ||
|
|
||
| public static final HunspellCacheInfoAction INSTANCE = new HunspellCacheInfoAction(); | ||
| public static final String NAME = "cluster:monitor/hunspell/cache"; | ||
|
|
||
| private HunspellCacheInfoAction() { | ||
| super(NAME, HunspellCacheInfoResponse::new); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| /* | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * The OpenSearch Contributors require contributions made to | ||
| * this file be licensed under the Apache-2.0 license or a | ||
| * compatible open source license. | ||
| */ | ||
|
|
||
| package org.opensearch.action.admin.indices.cache.hunspell; | ||
|
|
||
| import org.opensearch.action.ActionRequest; | ||
| import org.opensearch.action.ActionRequestValidationException; | ||
| import org.opensearch.core.common.io.stream.StreamInput; | ||
| import org.opensearch.core.common.io.stream.StreamOutput; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| /** | ||
| * Request for retrieving Hunspell cache information. | ||
| * | ||
| * @opensearch.internal | ||
| */ | ||
| public class HunspellCacheInfoRequest extends ActionRequest { | ||
|
|
||
| public HunspellCacheInfoRequest() {} | ||
|
|
||
| public HunspellCacheInfoRequest(StreamInput in) throws IOException { | ||
| super(in); | ||
| } | ||
|
|
||
| @Override | ||
| public void writeTo(StreamOutput out) throws IOException { | ||
| super.writeTo(out); | ||
| } | ||
|
|
||
| @Override | ||
| public ActionRequestValidationException validate() { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ActionModule's constructor signature now requires aHunspellService, but several in-repo callers still instantiateActionModulewith the old parameter list (e.g.,server/src/test/java/org/opensearch/action/ActionModuleTests.javaandserver/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java). This will fail compilation; please update those call sites to pass an appropriateHunspellService(ornullif you also make the parameter optional and handle it safely).