Skip to content

Commit 97aa359

Browse files
committed
Merge branch 'feature/maximus-m1' into maximus/add-system-table
2 parents 05a176e + 1bd0f35 commit 97aa359

File tree

17 files changed

+125
-212
lines changed

17 files changed

+125
-212
lines changed

core/src/main/java/org/opensearch/sql/expression/config/ExpressionConfig.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
import org.opensearch.sql.expression.operator.predicate.UnaryPredicateOperator;
2121
import org.opensearch.sql.expression.text.TextFunction;
2222
import org.opensearch.sql.expression.window.WindowFunctions;
23+
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
2324
import org.springframework.context.annotation.Bean;
2425
import org.springframework.context.annotation.Configuration;
26+
import org.springframework.context.annotation.Scope;
2527

2628
/**
2729
* Expression Config for Spring IoC.
@@ -32,6 +34,7 @@ public class ExpressionConfig {
3234
* BuiltinFunctionRepository constructor.
3335
*/
3436
@Bean
37+
@Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE)
3538
public BuiltinFunctionRepository functionRepository() {
3639
BuiltinFunctionRepository builtinFunctionRepository =
3740
new BuiltinFunctionRepository(new HashMap<>());
@@ -50,6 +53,7 @@ public BuiltinFunctionRepository functionRepository() {
5053
}
5154

5255
@Bean
56+
@Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE)
5357
public DSL dsl(BuiltinFunctionRepository repository) {
5458
return new DSL(repository);
5559
}

legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSQLQueryAction.java

Lines changed: 5 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,17 @@
1111
import static org.opensearch.sql.executor.ExecutionEngine.QueryResponse;
1212
import static org.opensearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY;
1313

14-
import java.io.IOException;
15-
import java.security.PrivilegedExceptionAction;
1614
import java.util.List;
17-
import javax.xml.catalog.Catalog;
1815
import org.apache.logging.log4j.LogManager;
1916
import org.apache.logging.log4j.Logger;
2017
import org.opensearch.client.node.NodeClient;
21-
import org.opensearch.cluster.service.ClusterService;
2218
import org.opensearch.rest.BaseRestHandler;
2319
import org.opensearch.rest.BytesRestResponse;
2420
import org.opensearch.rest.RestChannel;
2521
import org.opensearch.rest.RestRequest;
2622
import org.opensearch.rest.RestStatus;
27-
import org.opensearch.sql.catalog.CatalogService;
2823
import org.opensearch.sql.common.antlr.SyntaxCheckException;
2924
import org.opensearch.sql.common.response.ResponseListener;
30-
import org.opensearch.sql.common.setting.Settings;
3125
import org.opensearch.sql.executor.ExecutionEngine.ExplainResponse;
3226
import org.opensearch.sql.legacy.metrics.MetricName;
3327
import org.opensearch.sql.legacy.metrics.Metrics;
@@ -41,7 +35,6 @@
4135
import org.opensearch.sql.protocol.response.format.RawResponseFormatter;
4236
import org.opensearch.sql.protocol.response.format.ResponseFormatter;
4337
import org.opensearch.sql.sql.SQLService;
44-
import org.opensearch.sql.sql.config.SQLServiceConfig;
4538
import org.opensearch.sql.sql.domain.SQLQueryRequest;
4639
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
4740

@@ -56,23 +49,14 @@ public class RestSQLQueryAction extends BaseRestHandler {
5649

5750
public static final RestChannelConsumer NOT_SUPPORTED_YET = null;
5851

59-
private final ClusterService clusterService;
60-
61-
/**
62-
* Settings required by been initialization.
63-
*/
64-
private final Settings pluginSettings;
65-
66-
private final CatalogService catalogService;
52+
private final AnnotationConfigApplicationContext applicationContext;
6753

6854
/**
6955
* Constructor of RestSQLQueryAction.
7056
*/
71-
public RestSQLQueryAction(ClusterService clusterService, Settings pluginSettings, CatalogService catalogService) {
57+
public RestSQLQueryAction(AnnotationConfigApplicationContext applicationContext) {
7258
super();
73-
this.clusterService = clusterService;
74-
this.pluginSettings = pluginSettings;
75-
this.catalogService = catalogService;
59+
this.applicationContext = applicationContext;
7660
}
7761

7862
@Override
@@ -101,7 +85,8 @@ public RestChannelConsumer prepareRequest(SQLQueryRequest request, NodeClient no
10185
return NOT_SUPPORTED_YET;
10286
}
10387

104-
SQLService sqlService = createSQLService(nodeClient);
88+
SQLService sqlService =
89+
SecurityAccess.doPrivileged(() -> applicationContext.getBean(SQLService.class));
10590
PhysicalPlan plan;
10691
try {
10792
// For now analyzing and planning stage may throw syntax exception as well
@@ -123,20 +108,6 @@ public RestChannelConsumer prepareRequest(SQLQueryRequest request, NodeClient no
123108
return channel -> sqlService.execute(plan, createQueryResponseListener(channel, request));
124109
}
125110

126-
private SQLService createSQLService(NodeClient client) {
127-
return doPrivileged(() -> {
128-
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
129-
context.registerBean(ClusterService.class, () -> clusterService);
130-
context.registerBean(NodeClient.class, () -> client);
131-
context.registerBean(Settings.class, () -> pluginSettings);
132-
context.registerBean(CatalogService.class, () -> catalogService);
133-
context.register(OpenSearchSQLPluginConfig.class);
134-
context.register(SQLServiceConfig.class);
135-
context.refresh();
136-
return context.getBean(SQLService.class);
137-
});
138-
}
139-
140111
private ResponseListener<ExplainResponse> createExplainResponseListener(RestChannel channel) {
141112
return new ResponseListener<ExplainResponse>() {
142113
@Override
@@ -185,14 +156,6 @@ public void onFailure(Exception e) {
185156
};
186157
}
187158

188-
private <T> T doPrivileged(PrivilegedExceptionAction<T> action) {
189-
try {
190-
return SecurityAccess.doPrivileged(action);
191-
} catch (IOException e) {
192-
throw new IllegalStateException("Failed to perform privileged action", e);
193-
}
194-
}
195-
196159
private void sendResponse(RestChannel channel, RestStatus status, String content) {
197160
channel.sendResponse(new BytesRestResponse(
198161
status, "application/json; charset=UTF-8", content));

legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,13 @@
2727
import org.apache.logging.log4j.Logger;
2828
import org.opensearch.client.Client;
2929
import org.opensearch.client.node.NodeClient;
30-
import org.opensearch.cluster.service.ClusterService;
3130
import org.opensearch.common.settings.Settings;
3231
import org.opensearch.index.IndexNotFoundException;
3332
import org.opensearch.rest.BaseRestHandler;
3433
import org.opensearch.rest.BytesRestResponse;
3534
import org.opensearch.rest.RestChannel;
3635
import org.opensearch.rest.RestRequest;
3736
import org.opensearch.rest.RestStatus;
38-
import org.opensearch.sql.catalog.CatalogService;
3937
import org.opensearch.sql.common.antlr.SyntaxCheckException;
4038
import org.opensearch.sql.common.utils.QueryContext;
4139
import org.opensearch.sql.exception.ExpressionEvaluationException;
@@ -65,6 +63,7 @@
6563
import org.opensearch.sql.legacy.utils.JsonPrettyFormatter;
6664
import org.opensearch.sql.legacy.utils.QueryDataAnonymizer;
6765
import org.opensearch.sql.sql.domain.SQLQueryRequest;
66+
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
6867

6968
public class RestSqlAction extends BaseRestHandler {
7069

@@ -89,12 +88,16 @@ public class RestSqlAction extends BaseRestHandler {
8988
*/
9089
private final RestSQLQueryAction newSqlQueryHandler;
9190

92-
public RestSqlAction(Settings settings, ClusterService clusterService,
93-
org.opensearch.sql.common.setting.Settings pluginSettings,
94-
CatalogService catalogService) {
91+
/**
92+
* Application context used to create SQLService for each request.
93+
*/
94+
private final AnnotationConfigApplicationContext applicationContext;
95+
96+
public RestSqlAction(Settings settings, AnnotationConfigApplicationContext applicationContext) {
9597
super();
9698
this.allowExplicitIndex = MULTI_ALLOW_EXPLICIT_INDEX.get(settings);
97-
this.newSqlQueryHandler = new RestSQLQueryAction(clusterService, pluginSettings, catalogService);
99+
this.newSqlQueryHandler = new RestSQLQueryAction(applicationContext);
100+
this.applicationContext = applicationContext;
98101
}
99102

100103
@Override

legacy/src/test/java/org/opensearch/sql/legacy/plugin/RestSQLQueryActionTest.java

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
import static org.junit.Assert.assertNotSame;
1010
import static org.junit.Assert.assertSame;
11-
import static org.mockito.Mockito.when;
1211
import static org.opensearch.sql.legacy.plugin.RestSQLQueryAction.NOT_SUPPORTED_YET;
1312
import static org.opensearch.sql.legacy.plugin.RestSqlAction.EXPLAIN_API_ENDPOINT;
1413
import static org.opensearch.sql.legacy.plugin.RestSqlAction.QUERY_API_ENDPOINT;
@@ -18,36 +17,38 @@
1817
import org.junit.Test;
1918
import org.junit.runner.RunWith;
2019
import org.mockito.Mock;
20+
import org.mockito.Mockito;
2121
import org.mockito.junit.MockitoJUnitRunner;
2222
import org.opensearch.client.node.NodeClient;
23-
import org.opensearch.cluster.service.ClusterService;
2423
import org.opensearch.common.util.concurrent.ThreadContext;
2524
import org.opensearch.sql.catalog.CatalogService;
26-
import org.opensearch.sql.common.setting.Settings;
25+
import org.opensearch.sql.executor.ExecutionEngine;
26+
import org.opensearch.sql.sql.config.SQLServiceConfig;
2727
import org.opensearch.sql.sql.domain.SQLQueryRequest;
28+
import org.opensearch.sql.storage.StorageEngine;
2829
import org.opensearch.threadpool.ThreadPool;
30+
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
2931

3032
@RunWith(MockitoJUnitRunner.class)
3133
public class RestSQLQueryActionTest {
3234

33-
@Mock
34-
private ClusterService clusterService;
35-
3635
private NodeClient nodeClient;
3736

3837
@Mock
3938
private ThreadPool threadPool;
4039

41-
@Mock
42-
private Settings settings;
43-
44-
@Mock
45-
private CatalogService catalogService;
40+
private AnnotationConfigApplicationContext context;
4641

4742
@Before
4843
public void setup() {
4944
nodeClient = new NodeClient(org.opensearch.common.settings.Settings.EMPTY, threadPool);
50-
when(threadPool.getThreadContext())
45+
context = new AnnotationConfigApplicationContext();
46+
context.registerBean(StorageEngine.class, () -> Mockito.mock(StorageEngine.class));
47+
context.registerBean(ExecutionEngine.class, () -> Mockito.mock(ExecutionEngine.class));
48+
context.registerBean(CatalogService.class, () -> Mockito.mock(CatalogService.class));
49+
context.register(SQLServiceConfig.class);
50+
context.refresh();
51+
Mockito.lenient().when(threadPool.getThreadContext())
5152
.thenReturn(new ThreadContext(org.opensearch.common.settings.Settings.EMPTY));
5253
}
5354

@@ -59,7 +60,7 @@ public void handleQueryThatCanSupport() {
5960
QUERY_API_ENDPOINT,
6061
"");
6162

62-
RestSQLQueryAction queryAction = new RestSQLQueryAction(clusterService, settings, catalogService);
63+
RestSQLQueryAction queryAction = new RestSQLQueryAction(context);
6364
assertNotSame(NOT_SUPPORTED_YET, queryAction.prepareRequest(request, nodeClient));
6465
}
6566

@@ -71,7 +72,7 @@ public void handleExplainThatCanSupport() {
7172
EXPLAIN_API_ENDPOINT,
7273
"");
7374

74-
RestSQLQueryAction queryAction = new RestSQLQueryAction(clusterService, settings, catalogService);
75+
RestSQLQueryAction queryAction = new RestSQLQueryAction(context);
7576
assertNotSame(NOT_SUPPORTED_YET, queryAction.prepareRequest(request, nodeClient));
7677
}
7778

@@ -84,7 +85,7 @@ public void skipQueryThatNotSupport() {
8485
QUERY_API_ENDPOINT,
8586
"");
8687

87-
RestSQLQueryAction queryAction = new RestSQLQueryAction(clusterService, settings, catalogService);
88+
RestSQLQueryAction queryAction = new RestSQLQueryAction(context);
8889
assertSame(NOT_SUPPORTED_YET, queryAction.prepareRequest(request, nodeClient));
8990
}
9091

opensearch/src/main/java/org/opensearch/sql/opensearch/security/SecurityAccess.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
package org.opensearch.sql.opensearch.security;
88

9-
import java.io.IOException;
109
import java.security.AccessController;
1110
import java.security.PrivilegedActionException;
1211
import java.security.PrivilegedExceptionAction;
@@ -21,13 +20,12 @@ public class SecurityAccess {
2120
/**
2221
* Execute the operation in privileged mode.
2322
*/
24-
public static <T> T doPrivileged(final PrivilegedExceptionAction<T> operation)
25-
throws IOException {
23+
public static <T> T doPrivileged(final PrivilegedExceptionAction<T> operation) {
2624
SpecialPermission.check();
2725
try {
2826
return AccessController.doPrivileged(operation);
2927
} catch (final PrivilegedActionException e) {
30-
throw (IOException) e.getCause();
28+
throw new IllegalStateException("Failed to perform privileged action", e);
3129
}
3230
}
3331
}

0 commit comments

Comments
 (0)